| Message ID | 20240813074233.2473876-5-danishanwar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce HSR offload support for ICSSG | expand |
On Tue, Aug 13, 2024 at 01:12:30PM +0530, MD Danish Anwar wrote: > Add support for offloading HSR port-to-port frame forward to hardware. > When the slave interfaces are added to the HSR interface, the PRU cores > will be stopped and ICSSG HSR firmwares will be loaded to them. > > Similarly, when HSR interface is deleted, the PRU cores will be stopped > and dual EMAC firmware will be loaded to them. Maybe a dumb question, because i don't know HSR.... Can you have one interface in a HSR network, another interface in a non-HSR network, and bridge packets between the two worlds? Do you want the HSR firmware, the Switchdev firmware, or Dual EMAC and do the bridge in software? > void icssg_class_set_mac_addr(struct regmap *miig_rt, int slice, u8 *mac) > { > diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c > index dae52a83a378..2f485318c940 100644 > --- a/drivers/net/ethernet/ti/icssg/icssg_config.c > +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c > @@ -455,7 +455,7 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice) > struct icssg_flow_cfg __iomem *flow_cfg; > int ret; > > - if (prueth->is_switch_mode) > + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) > icssg_init_switch_mode(prueth); Maybe icssg_init_switch_mode() needs renaming if it is used for more than switch mode? There are other functions which might need generalising. > +#define NETIF_PRUETH_HSR_OFFLOAD NETIF_F_HW_HSR_FWD > + > /* CTRLMMR_ICSSG_RGMII_CTRL register bits */ > #define ICSSG_CTRL_RGMII_ID_MODE BIT(24) > > @@ -118,6 +121,19 @@ static irqreturn_t prueth_tx_ts_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static struct icssg_firmwares icssg_hsr_firmwares[] = { > + { > + .pru = "ti-pruss/am65x-sr2-pru0-pruhsr-fw.elf", > + .rtu = "ti-pruss/am65x-sr2-rtu0-pruhsr-fw.elf", > + .txpru = "ti-pruss/am65x-sr2-txpru0-pruhsr-fw.elf", > + }, > + { > + .pru = "ti-pruss/am65x-sr2-pru1-pruhsr-fw.elf", > + .rtu = "ti-pruss/am65x-sr2-rtu1-pruhsr-fw.elf", > + .txpru = "ti-pruss/am65x-sr2-txpru1-pruhsr-fw.elf", > + } > +}; > + > static struct icssg_firmwares icssg_switch_firmwares[] = { > { > .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", > @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) > > if (prueth->is_switch_mode) > firmwares = icssg_switch_firmwares; > + else if (prueth->is_hsr_offload_mode) > + firmwares = icssg_hsr_firmwares; Documentation/networking/netdev-features.rst * hsr-fwd-offload This should be set for devices which forward HSR (High-availability Seamless Redundancy) frames from one port to another in hardware. To me, this suggests if the flag is not set, you should keep in dual EMACS or switchdev mode and perform HSR in software. > +static int emac_ndo_set_features(struct net_device *ndev, > + netdev_features_t features) > +{ > + netdev_features_t hsr_feature_present = ndev->features & NETIF_PRUETH_HSR_OFFLOAD; > + netdev_features_t hsr_feature_wanted = features & NETIF_PRUETH_HSR_OFFLOAD; I would not add the _PRUETH_ alias. There is nothing _PRUETH_ specific here, its just plain HSR offload. > +static int prueth_hsr_port_link(struct net_device *ndev) > +{ > + struct prueth_emac *emac = netdev_priv(ndev); > + struct prueth *prueth = emac->prueth; > + struct prueth_emac *emac0; > + struct prueth_emac *emac1; > + > + emac0 = prueth->emac[PRUETH_MAC0]; > + emac1 = prueth->emac[PRUETH_MAC1]; > + > + if (prueth->is_switch_mode) { > + dev_err(prueth->dev, "Switching from bridge to HSR mode not allowed\n"); > + return -EINVAL; I think you want EOPNOTSUPP, so that it is performed in software, not offloaded to hardware. And this is not an error condition, it is just a limitation of your hardware/firmware. > + prueth->hsr_members |= BIT(emac->port_id); > + if (!prueth->is_switch_mode && !prueth->is_hsr_offload_mode) { > + if (prueth->hsr_members & BIT(PRUETH_PORT_MII0) && > + prueth->hsr_members & BIT(PRUETH_PORT_MII1)) { > + if (!(emac0->ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && > + !(emac1->ndev->features & NETIF_PRUETH_HSR_OFFLOAD)) { > + dev_err(prueth->dev, "Enable HSR offload on both interfaces\n"); > + return -EINVAL; Again, EOPNOTSUPP, so it falls back to software, and no dev_err(). > + } > + prueth->is_hsr_offload_mode = true; > + prueth->default_vlan = 1; > + emac0->port_vlan = prueth->default_vlan; > + emac1->port_vlan = prueth->default_vlan; > + icssg_change_mode(prueth); > + dev_err(prueth->dev, "Enabling HSR offload mode\n"); This is not an error condition. dev_dbg(). > + } > + } > + > + return 0; > +} > + > +static void prueth_hsr_port_unlink(struct net_device *ndev) > +{ > + struct prueth_emac *emac = netdev_priv(ndev); > + struct prueth *prueth = emac->prueth; > + struct prueth_emac *emac0; > + struct prueth_emac *emac1; > + > + emac0 = prueth->emac[PRUETH_MAC0]; > + emac1 = prueth->emac[PRUETH_MAC1]; > + > + prueth->hsr_members &= ~BIT(emac->port_id); > + if (prueth->is_hsr_offload_mode) { > + prueth->is_hsr_offload_mode = false; > + emac0->port_vlan = 0; > + emac1->port_vlan = 0; > + prueth->hsr_dev = NULL; > + prueth_emac_restart(prueth); > + dev_info(prueth->dev, "Enabling Dual EMAC mode\n"); dev_dbg(). > + } > +} > + > /* netdev notifier */ > static int prueth_netdevice_event(struct notifier_block *unused, > unsigned long event, void *ptr) > @@ -1047,6 +1141,8 @@ static int prueth_netdevice_event(struct notifier_block *unused, > struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); > struct net_device *ndev = netdev_notifier_info_to_dev(ptr); > struct netdev_notifier_changeupper_info *info; > + struct prueth_emac *emac = netdev_priv(ndev); > + struct prueth *prueth = emac->prueth; > int ret = NOTIFY_DONE; > > if (ndev->netdev_ops != &emac_netdev_ops) > @@ -1056,6 +1152,26 @@ static int prueth_netdevice_event(struct notifier_block *unused, > case NETDEV_CHANGEUPPER: > info = ptr; > > + if ((ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && > + is_hsr_master(info->upper_dev)) { > + if (info->linking) { > + if (!prueth->hsr_dev) { > + prueth->hsr_dev = info->upper_dev; > + > + icssg_class_set_host_mac_addr(prueth->miig_rt, > + prueth->hsr_dev->dev_addr); > + } else { > + if (prueth->hsr_dev != info->upper_dev) { > + dev_err(prueth->dev, "Both interfaces must be linked to same upper device\n"); dev_dbg() Andrew
On 13/08/24 8:47 pm, Andrew Lunn wrote: > On Tue, Aug 13, 2024 at 01:12:30PM +0530, MD Danish Anwar wrote: >> Add support for offloading HSR port-to-port frame forward to hardware. >> When the slave interfaces are added to the HSR interface, the PRU cores >> will be stopped and ICSSG HSR firmwares will be loaded to them. >> >> Similarly, when HSR interface is deleted, the PRU cores will be stopped >> and dual EMAC firmware will be loaded to them. > > Maybe a dumb question, because i don't know HSR.... > > Can you have one interface in a HSR network, another interface in a > non-HSR network, and bridge packets between the two worlds? Do you > want the HSR firmware, the Switchdev firmware, or Dual EMAC and do the > bridge in software? > As far as I know, when adding an hsr interface we need to specify both the slave interfaces ` ip link add name hsr0 type hsr slave1 eth1 slave2 eth2 supervision 45 version 1` HSR is only enabled when both the ports are added to hsr interface. If hsr-fwd-offload is set, the firmware will be changed to HSR otherwise Dual EMAC firmware will keep running and hsr forwarding will happen in software. >> void icssg_class_set_mac_addr(struct regmap *miig_rt, int slice, u8 *mac) >> { >> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c >> index dae52a83a378..2f485318c940 100644 >> --- a/drivers/net/ethernet/ti/icssg/icssg_config.c >> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c >> @@ -455,7 +455,7 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice) >> struct icssg_flow_cfg __iomem *flow_cfg; >> int ret; >> >> - if (prueth->is_switch_mode) >> + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) >> icssg_init_switch_mode(prueth); > > Maybe icssg_init_switch_mode() needs renaming if it is used for more > than switch mode? There are other functions which might need > generalising. > Yes, the icssg_init_ and many other APIs are common for switch and hsr. They can be renamed to indicate that as well. How does icssg_init_switch_or_hsr_mode() sound? >> +#define NETIF_PRUETH_HSR_OFFLOAD NETIF_F_HW_HSR_FWD >> + >> /* CTRLMMR_ICSSG_RGMII_CTRL register bits */ >> #define ICSSG_CTRL_RGMII_ID_MODE BIT(24) >> >> @@ -118,6 +121,19 @@ static irqreturn_t prueth_tx_ts_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> >> +static struct icssg_firmwares icssg_hsr_firmwares[] = { >> + { >> + .pru = "ti-pruss/am65x-sr2-pru0-pruhsr-fw.elf", >> + .rtu = "ti-pruss/am65x-sr2-rtu0-pruhsr-fw.elf", >> + .txpru = "ti-pruss/am65x-sr2-txpru0-pruhsr-fw.elf", >> + }, >> + { >> + .pru = "ti-pruss/am65x-sr2-pru1-pruhsr-fw.elf", >> + .rtu = "ti-pruss/am65x-sr2-rtu1-pruhsr-fw.elf", >> + .txpru = "ti-pruss/am65x-sr2-txpru1-pruhsr-fw.elf", >> + } >> +}; >> + >> static struct icssg_firmwares icssg_switch_firmwares[] = { >> { >> .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", >> @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) >> >> if (prueth->is_switch_mode) >> firmwares = icssg_switch_firmwares; >> + else if (prueth->is_hsr_offload_mode) >> + firmwares = icssg_hsr_firmwares; > > Documentation/networking/netdev-features.rst > > * hsr-fwd-offload > > This should be set for devices which forward HSR (High-availability Seamless > Redundancy) frames from one port to another in hardware. > > To me, this suggests if the flag is not set, you should keep in dual > EMACS or switchdev mode and perform HSR in software. Correct. This is the expected behavior. If the flag is not set we remain in dual EMAC firmware and do HSR in software. Please see prueth_hsr_port_link() for detail on this. > >> +static int emac_ndo_set_features(struct net_device *ndev, >> + netdev_features_t features) >> +{ >> + netdev_features_t hsr_feature_present = ndev->features & NETIF_PRUETH_HSR_OFFLOAD; >> + netdev_features_t hsr_feature_wanted = features & NETIF_PRUETH_HSR_OFFLOAD; > > I would not add the _PRUETH_ alias. There is nothing _PRUETH_ specific > here, its just plain HSR offload. > I see your query in this is resolved by https://lore.kernel.org/all/985e10e4-49df-46d8-b9c2-d385dab569a9@lunn.ch/ >> +static int prueth_hsr_port_link(struct net_device *ndev) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + struct prueth *prueth = emac->prueth; >> + struct prueth_emac *emac0; >> + struct prueth_emac *emac1; >> + >> + emac0 = prueth->emac[PRUETH_MAC0]; >> + emac1 = prueth->emac[PRUETH_MAC1]; >> + >> + if (prueth->is_switch_mode) { >> + dev_err(prueth->dev, "Switching from bridge to HSR mode not allowed\n"); >> + return -EINVAL; > > I think you want EOPNOTSUPP, so that it is performed in software, not > offloaded to hardware. And this is not an error condition, it is just > a limitation of your hardware/firmware. > Sure. >> + prueth->hsr_members |= BIT(emac->port_id); >> + if (!prueth->is_switch_mode && !prueth->is_hsr_offload_mode) { >> + if (prueth->hsr_members & BIT(PRUETH_PORT_MII0) && >> + prueth->hsr_members & BIT(PRUETH_PORT_MII1)) { >> + if (!(emac0->ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && >> + !(emac1->ndev->features & NETIF_PRUETH_HSR_OFFLOAD)) { >> + dev_err(prueth->dev, "Enable HSR offload on both interfaces\n"); >> + return -EINVAL; > > Again, EOPNOTSUPP, so it falls back to software, and no dev_err(). sure I will change this to EOPNOTSUPP and remove the print. > >> + } >> + prueth->is_hsr_offload_mode = true; >> + prueth->default_vlan = 1; >> + emac0->port_vlan = prueth->default_vlan; >> + emac1->port_vlan = prueth->default_vlan; >> + icssg_change_mode(prueth); >> + dev_err(prueth->dev, "Enabling HSR offload mode\n"); > > This is not an error condition. dev_dbg(). Sure. > >> + } >> + } >> + >> + return 0; >> +} >> + >> +static void prueth_hsr_port_unlink(struct net_device *ndev) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + struct prueth *prueth = emac->prueth; >> + struct prueth_emac *emac0; >> + struct prueth_emac *emac1; >> + >> + emac0 = prueth->emac[PRUETH_MAC0]; >> + emac1 = prueth->emac[PRUETH_MAC1]; >> + >> + prueth->hsr_members &= ~BIT(emac->port_id); >> + if (prueth->is_hsr_offload_mode) { >> + prueth->is_hsr_offload_mode = false; >> + emac0->port_vlan = 0; >> + emac1->port_vlan = 0; >> + prueth->hsr_dev = NULL; >> + prueth_emac_restart(prueth); >> + dev_info(prueth->dev, "Enabling Dual EMAC mode\n"); > > dev_dbg(). Sure. > >> + } >> +} >> + >> /* netdev notifier */ >> static int prueth_netdevice_event(struct notifier_block *unused, >> unsigned long event, void *ptr) >> @@ -1047,6 +1141,8 @@ static int prueth_netdevice_event(struct notifier_block *unused, >> struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); >> struct net_device *ndev = netdev_notifier_info_to_dev(ptr); >> struct netdev_notifier_changeupper_info *info; >> + struct prueth_emac *emac = netdev_priv(ndev); >> + struct prueth *prueth = emac->prueth; >> int ret = NOTIFY_DONE; >> >> if (ndev->netdev_ops != &emac_netdev_ops) >> @@ -1056,6 +1152,26 @@ static int prueth_netdevice_event(struct notifier_block *unused, >> case NETDEV_CHANGEUPPER: >> info = ptr; >> >> + if ((ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && >> + is_hsr_master(info->upper_dev)) { >> + if (info->linking) { >> + if (!prueth->hsr_dev) { >> + prueth->hsr_dev = info->upper_dev; >> + >> + icssg_class_set_host_mac_addr(prueth->miig_rt, >> + prueth->hsr_dev->dev_addr); >> + } else { >> + if (prueth->hsr_dev != info->upper_dev) { >> + dev_err(prueth->dev, "Both interfaces must be linked to same upper device\n"); > > dev_dbg() Sure. I will do these changes and send out a new version. Please let me know if any other change is needed. > > Andrew
> Yes, the icssg_init_ and many other APIs are common for switch and hsr. > They can be renamed to indicate that as well. > > How does icssg_init_switch_or_hsr_mode() sound? I would say it is too long. And when you add the next thing, say bonding, will it become icssg_init_switch_or_hsr_or_bond_mode()? Maybe name the function after what it actually does, not why you call it. > >> static struct icssg_firmwares icssg_switch_firmwares[] = { > >> { > >> .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", > >> @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) > >> > >> if (prueth->is_switch_mode) > >> firmwares = icssg_switch_firmwares; > >> + else if (prueth->is_hsr_offload_mode) > >> + firmwares = icssg_hsr_firmwares; > > > > Documentation/networking/netdev-features.rst > > > > * hsr-fwd-offload > > > > This should be set for devices which forward HSR (High-availability Seamless > > Redundancy) frames from one port to another in hardware. > > > > To me, this suggests if the flag is not set, you should keep in dual > > EMACS or switchdev mode and perform HSR in software. > > > Correct. This is the expected behavior. If the flag is not set we remain > in dual EMAC firmware and do HSR in software. Please see > prueth_hsr_port_link() for detail on this. O.K. Andrew
On 8/14/2024 7:32 PM, Andrew Lunn wrote: >> Yes, the icssg_init_ and many other APIs are common for switch and hsr. >> They can be renamed to indicate that as well. >> >> How does icssg_init_switch_or_hsr_mode() sound? > > I would say it is too long. And when you add the next thing, say > bonding, will it become icssg_init_switch_or_hsr_or_bond_mode()? > > Maybe name the function after what it actually does, not why you call > it. > Sure Andrew, I will try to come up with a proper name for these APIs. >>>> static struct icssg_firmwares icssg_switch_firmwares[] = { >>>> { >>>> .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", >>>> @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) >>>> >>>> if (prueth->is_switch_mode) >>>> firmwares = icssg_switch_firmwares; >>>> + else if (prueth->is_hsr_offload_mode) >>>> + firmwares = icssg_hsr_firmwares; >>> >>> Documentation/networking/netdev-features.rst >>> >>> * hsr-fwd-offload >>> >>> This should be set for devices which forward HSR (High-availability Seamless >>> Redundancy) frames from one port to another in hardware. >>> >>> To me, this suggests if the flag is not set, you should keep in dual >>> EMACS or switchdev mode and perform HSR in software. >> >> >> Correct. This is the expected behavior. If the flag is not set we remain >> in dual EMAC firmware and do HSR in software. Please see >> prueth_hsr_port_link() for detail on this. > > O.K. > > Andrew
On Tue, Aug 13, 2024 at 01:12:30PM +0530, MD Danish Anwar wrote: ... > diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h > index f678d656a3ed..40bc3912b6ae 100644 > --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h > +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h > @@ -239,6 +239,7 @@ struct icssg_firmwares { > * @iep1: pointer to IEP1 device > * @vlan_tbl: VLAN-FID table pointer > * @hw_bridge_dev: pointer to HW bridge net device > + * @hsr_dev: pointer to the HSR net device > * @br_members: bitmask of bridge member ports > * @prueth_netdevice_nb: netdevice notifier block > * @prueth_switchdev_nb: switchdev notifier block I think you also need to add Kernel doc entries for @hsr_members and @is_hsr_offload_mode. Flagged by W=1 builds and ./scripts/kernel-doc -none > @@ -274,11 +275,14 @@ struct prueth { > struct prueth_vlan_tbl *vlan_tbl; > > struct net_device *hw_bridge_dev; > + struct net_device *hsr_dev; > u8 br_members; > + u8 hsr_members; > struct notifier_block prueth_netdevice_nb; > struct notifier_block prueth_switchdev_nb; > struct notifier_block prueth_switchdev_bl_nb; > bool is_switch_mode; > + bool is_hsr_offload_mode; > bool is_switchmode_supported; > unsigned char switch_id[MAX_PHYS_ITEM_ID_LEN]; > int default_vlan; > -- > 2.34.1 >
On 8/15/2024 8:44 PM, Simon Horman wrote: > On Tue, Aug 13, 2024 at 01:12:30PM +0530, MD Danish Anwar wrote: > > ... > >> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h >> index f678d656a3ed..40bc3912b6ae 100644 >> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h >> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h >> @@ -239,6 +239,7 @@ struct icssg_firmwares { >> * @iep1: pointer to IEP1 device >> * @vlan_tbl: VLAN-FID table pointer >> * @hw_bridge_dev: pointer to HW bridge net device >> + * @hsr_dev: pointer to the HSR net device >> * @br_members: bitmask of bridge member ports >> * @prueth_netdevice_nb: netdevice notifier block >> * @prueth_switchdev_nb: switchdev notifier block > > I think you also need to add Kernel doc entries for @hsr_members and > @is_hsr_offload_mode. > > Flagged by W=1 builds and ./scripts/kernel-doc -none Sure Simon, I'll do that. > >> @@ -274,11 +275,14 @@ struct prueth { >> struct prueth_vlan_tbl *vlan_tbl; >> >> struct net_device *hw_bridge_dev; >> + struct net_device *hsr_dev; >> u8 br_members; >> + u8 hsr_members; >> struct notifier_block prueth_netdevice_nb; >> struct notifier_block prueth_switchdev_nb; >> struct notifier_block prueth_switchdev_bl_nb; >> bool is_switch_mode; >> + bool is_hsr_offload_mode; >> bool is_switchmode_supported; >> unsigned char switch_id[MAX_PHYS_ITEM_ID_LEN]; >> int default_vlan; >> -- >> 2.34.1 >>
On 8/14/2024 8:24 PM, Anwar, Md Danish wrote: > > > On 8/14/2024 7:32 PM, Andrew Lunn wrote: >>> Yes, the icssg_init_ and many other APIs are common for switch and hsr. >>> They can be renamed to indicate that as well. >>> >>> How does icssg_init_switch_or_hsr_mode() sound? >> >> I would say it is too long. And when you add the next thing, say >> bonding, will it become icssg_init_switch_or_hsr_or_bond_mode()? >> >> Maybe name the function after what it actually does, not why you call >> it. >> Andrew, the functions are actually doing some configurations different than EMAC mode which are needed for offloading the frames for switch as well as HSR mode. icssg_init_emac_mode() doesn't do any configuration related to offloading. Perhaps naming these APIs to icssg_init_fw_offload_mode() will be a better fit? Other Similar APIs can also be changed to use _fw_offload instead of _switch. How does this sound? > Sure Andrew, I will try to come up with a proper name for these APIs. > >>>>> static struct icssg_firmwares icssg_switch_firmwares[] = { >>>>> { >>>>> .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", >>>>> @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) >>>>> >>>>> if (prueth->is_switch_mode) >>>>> firmwares = icssg_switch_firmwares; >>>>> + else if (prueth->is_hsr_offload_mode) >>>>> + firmwares = icssg_hsr_firmwares; >>>> >>>> Documentation/networking/netdev-features.rst >>>> >>>> * hsr-fwd-offload >>>> >>>> This should be set for devices which forward HSR (High-availability Seamless >>>> Redundancy) frames from one port to another in hardware. >>>> >>>> To me, this suggests if the flag is not set, you should keep in dual >>>> EMACS or switchdev mode and perform HSR in software. >>> >>> >>> Correct. This is the expected behavior. If the flag is not set we remain >>> in dual EMAC firmware and do HSR in software. Please see >>> prueth_hsr_port_link() for detail on this. >> >> O.K. >> >> Andrew >
diff --git a/drivers/net/ethernet/ti/icssg/icssg_classifier.c b/drivers/net/ethernet/ti/icssg/icssg_classifier.c index 9ec504d976d6..833ca86d0b71 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_classifier.c +++ b/drivers/net/ethernet/ti/icssg/icssg_classifier.c @@ -290,6 +290,7 @@ void icssg_class_set_host_mac_addr(struct regmap *miig_rt, const u8 *mac) mac[2] << 16 | mac[3] << 24)); regmap_write(miig_rt, MAC_INTERFACE_1, (u32)(mac[4] | mac[5] << 8)); } +EXPORT_SYMBOL_GPL(icssg_class_set_host_mac_addr); void icssg_class_set_mac_addr(struct regmap *miig_rt, int slice, u8 *mac) { diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.c b/drivers/net/ethernet/ti/icssg/icssg_config.c index dae52a83a378..2f485318c940 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_config.c +++ b/drivers/net/ethernet/ti/icssg/icssg_config.c @@ -455,7 +455,7 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice) struct icssg_flow_cfg __iomem *flow_cfg; int ret; - if (prueth->is_switch_mode) + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) icssg_init_switch_mode(prueth); else icssg_init_emac_mode(prueth); @@ -472,7 +472,7 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice) regmap_update_bits(prueth->miig_rt, ICSSG_CFG_OFFSET, ICSSG_CFG_DEFAULT, ICSSG_CFG_DEFAULT); icssg_miig_set_interface_mode(prueth->miig_rt, slice, emac->phy_if); - if (prueth->is_switch_mode) + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) icssg_config_mii_init_switch(emac); else icssg_config_mii_init(emac); @@ -498,7 +498,7 @@ int icssg_config(struct prueth *prueth, struct prueth_emac *emac, int slice) writeb(0, config + SPL_PKT_DEFAULT_PRIORITY); writeb(0, config + QUEUE_NUM_UNTAGGED); - if (prueth->is_switch_mode) + if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) ret = prueth_switch_buffer_setup(emac); else ret = prueth_emac_buffer_setup(emac); diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c index c93071e05c37..142e267ff136 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c @@ -13,6 +13,7 @@ #include <linux/dma/ti-cppi5.h> #include <linux/etherdevice.h> #include <linux/genalloc.h> +#include <linux/if_hsr.h> #include <linux/if_vlan.h> #include <linux/interrupt.h> #include <linux/kernel.h> @@ -40,6 +41,8 @@ #define DEFAULT_PORT_MASK 1 #define DEFAULT_UNTAG_MASK 1 +#define NETIF_PRUETH_HSR_OFFLOAD NETIF_F_HW_HSR_FWD + /* CTRLMMR_ICSSG_RGMII_CTRL register bits */ #define ICSSG_CTRL_RGMII_ID_MODE BIT(24) @@ -118,6 +121,19 @@ static irqreturn_t prueth_tx_ts_irq(int irq, void *dev_id) return IRQ_HANDLED; } +static struct icssg_firmwares icssg_hsr_firmwares[] = { + { + .pru = "ti-pruss/am65x-sr2-pru0-pruhsr-fw.elf", + .rtu = "ti-pruss/am65x-sr2-rtu0-pruhsr-fw.elf", + .txpru = "ti-pruss/am65x-sr2-txpru0-pruhsr-fw.elf", + }, + { + .pru = "ti-pruss/am65x-sr2-pru1-pruhsr-fw.elf", + .rtu = "ti-pruss/am65x-sr2-rtu1-pruhsr-fw.elf", + .txpru = "ti-pruss/am65x-sr2-txpru1-pruhsr-fw.elf", + } +}; + static struct icssg_firmwares icssg_switch_firmwares[] = { { .pru = "ti-pruss/am65x-sr2-pru0-prusw-fw.elf", @@ -152,6 +168,8 @@ static int prueth_emac_start(struct prueth *prueth, struct prueth_emac *emac) if (prueth->is_switch_mode) firmwares = icssg_switch_firmwares; + else if (prueth->is_hsr_offload_mode) + firmwares = icssg_hsr_firmwares; else firmwares = icssg_emac_firmwares; @@ -726,6 +744,19 @@ static void emac_ndo_set_rx_mode(struct net_device *ndev) queue_work(emac->cmd_wq, &emac->rx_mode_work); } +static int emac_ndo_set_features(struct net_device *ndev, + netdev_features_t features) +{ + netdev_features_t hsr_feature_present = ndev->features & NETIF_PRUETH_HSR_OFFLOAD; + netdev_features_t hsr_feature_wanted = features & NETIF_PRUETH_HSR_OFFLOAD; + bool hsr_change_request = ((hsr_feature_wanted ^ hsr_feature_present) != 0); + + if (hsr_change_request) + ndev->features = features; + + return 0; +} + static const struct net_device_ops emac_netdev_ops = { .ndo_open = emac_ndo_open, .ndo_stop = emac_ndo_stop, @@ -737,6 +768,7 @@ static const struct net_device_ops emac_netdev_ops = { .ndo_eth_ioctl = icssg_ndo_ioctl, .ndo_get_stats64 = icssg_ndo_get_stats64, .ndo_get_phys_port_name = icssg_ndo_get_phys_port_name, + .ndo_set_features = emac_ndo_set_features, }; static int prueth_netdev_init(struct prueth *prueth, @@ -865,6 +897,7 @@ static int prueth_netdev_init(struct prueth *prueth, ndev->ethtool_ops = &icssg_ethtool_ops; ndev->hw_features = NETIF_F_SG; ndev->features = ndev->hw_features; + ndev->hw_features |= NETIF_F_HW_HSR_FWD; netif_napi_add(ndev, &emac->napi_rx, icssg_napi_rx_poll); hrtimer_init(&emac->rx_hrtimer, CLOCK_MONOTONIC, @@ -953,7 +986,7 @@ static void prueth_emac_restart(struct prueth *prueth) netif_device_attach(emac1->ndev); } -static void icssg_enable_switch_mode(struct prueth *prueth) +static void icssg_change_mode(struct prueth *prueth) { struct prueth_emac *emac; int mac; @@ -973,8 +1006,12 @@ static void icssg_enable_switch_mode(struct prueth *prueth) BIT(emac->port_id) | DEFAULT_PORT_MASK, BIT(emac->port_id) | DEFAULT_UNTAG_MASK, true); + if (prueth->is_hsr_offload_mode) + icssg_vtbl_modify(emac, DEFAULT_VID, DEFAULT_PORT_MASK, + DEFAULT_UNTAG_MASK, true); icssg_set_pvid(prueth, emac->port_vlan, emac->port_id); - icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE); + if (prueth->is_switch_mode) + icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE); } } } @@ -1012,7 +1049,7 @@ static int prueth_netdevice_port_link(struct net_device *ndev, prueth->is_switch_mode = true; prueth->default_vlan = 1; emac->port_vlan = prueth->default_vlan; - icssg_enable_switch_mode(prueth); + icssg_change_mode(prueth); } } @@ -1040,6 +1077,63 @@ static void prueth_netdevice_port_unlink(struct net_device *ndev) prueth->hw_bridge_dev = NULL; } +static int prueth_hsr_port_link(struct net_device *ndev) +{ + struct prueth_emac *emac = netdev_priv(ndev); + struct prueth *prueth = emac->prueth; + struct prueth_emac *emac0; + struct prueth_emac *emac1; + + emac0 = prueth->emac[PRUETH_MAC0]; + emac1 = prueth->emac[PRUETH_MAC1]; + + if (prueth->is_switch_mode) { + dev_err(prueth->dev, "Switching from bridge to HSR mode not allowed\n"); + return -EINVAL; + } + + prueth->hsr_members |= BIT(emac->port_id); + if (!prueth->is_switch_mode && !prueth->is_hsr_offload_mode) { + if (prueth->hsr_members & BIT(PRUETH_PORT_MII0) && + prueth->hsr_members & BIT(PRUETH_PORT_MII1)) { + if (!(emac0->ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && + !(emac1->ndev->features & NETIF_PRUETH_HSR_OFFLOAD)) { + dev_err(prueth->dev, "Enable HSR offload on both interfaces\n"); + return -EINVAL; + } + prueth->is_hsr_offload_mode = true; + prueth->default_vlan = 1; + emac0->port_vlan = prueth->default_vlan; + emac1->port_vlan = prueth->default_vlan; + icssg_change_mode(prueth); + dev_err(prueth->dev, "Enabling HSR offload mode\n"); + } + } + + return 0; +} + +static void prueth_hsr_port_unlink(struct net_device *ndev) +{ + struct prueth_emac *emac = netdev_priv(ndev); + struct prueth *prueth = emac->prueth; + struct prueth_emac *emac0; + struct prueth_emac *emac1; + + emac0 = prueth->emac[PRUETH_MAC0]; + emac1 = prueth->emac[PRUETH_MAC1]; + + prueth->hsr_members &= ~BIT(emac->port_id); + if (prueth->is_hsr_offload_mode) { + prueth->is_hsr_offload_mode = false; + emac0->port_vlan = 0; + emac1->port_vlan = 0; + prueth->hsr_dev = NULL; + prueth_emac_restart(prueth); + dev_info(prueth->dev, "Enabling Dual EMAC mode\n"); + } +} + /* netdev notifier */ static int prueth_netdevice_event(struct notifier_block *unused, unsigned long event, void *ptr) @@ -1047,6 +1141,8 @@ static int prueth_netdevice_event(struct notifier_block *unused, struct netlink_ext_ack *extack = netdev_notifier_info_to_extack(ptr); struct net_device *ndev = netdev_notifier_info_to_dev(ptr); struct netdev_notifier_changeupper_info *info; + struct prueth_emac *emac = netdev_priv(ndev); + struct prueth *prueth = emac->prueth; int ret = NOTIFY_DONE; if (ndev->netdev_ops != &emac_netdev_ops) @@ -1056,6 +1152,26 @@ static int prueth_netdevice_event(struct notifier_block *unused, case NETDEV_CHANGEUPPER: info = ptr; + if ((ndev->features & NETIF_PRUETH_HSR_OFFLOAD) && + is_hsr_master(info->upper_dev)) { + if (info->linking) { + if (!prueth->hsr_dev) { + prueth->hsr_dev = info->upper_dev; + + icssg_class_set_host_mac_addr(prueth->miig_rt, + prueth->hsr_dev->dev_addr); + } else { + if (prueth->hsr_dev != info->upper_dev) { + dev_err(prueth->dev, "Both interfaces must be linked to same upper device\n"); + return -EOPNOTSUPP; + } + } + prueth_hsr_port_link(ndev); + } else { + prueth_hsr_port_unlink(ndev); + } + } + if (netif_is_bridge_master(info->upper_dev)) { if (info->linking) ret = prueth_netdevice_port_link(ndev, info->upper_dev, extack); diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h index f678d656a3ed..40bc3912b6ae 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h @@ -239,6 +239,7 @@ struct icssg_firmwares { * @iep1: pointer to IEP1 device * @vlan_tbl: VLAN-FID table pointer * @hw_bridge_dev: pointer to HW bridge net device + * @hsr_dev: pointer to the HSR net device * @br_members: bitmask of bridge member ports * @prueth_netdevice_nb: netdevice notifier block * @prueth_switchdev_nb: switchdev notifier block @@ -274,11 +275,14 @@ struct prueth { struct prueth_vlan_tbl *vlan_tbl; struct net_device *hw_bridge_dev; + struct net_device *hsr_dev; u8 br_members; + u8 hsr_members; struct notifier_block prueth_netdevice_nb; struct notifier_block prueth_switchdev_nb; struct notifier_block prueth_switchdev_bl_nb; bool is_switch_mode; + bool is_hsr_offload_mode; bool is_switchmode_supported; unsigned char switch_id[MAX_PHYS_ITEM_ID_LEN]; int default_vlan;
Add support for offloading HSR port-to-port frame forward to hardware. When the slave interfaces are added to the HSR interface, the PRU cores will be stopped and ICSSG HSR firmwares will be loaded to them. Similarly, when HSR interface is deleted, the PRU cores will be stopped and dual EMAC firmware will be loaded to them. Signed-off-by: MD Danish Anwar <danishanwar@ti.com> --- .../net/ethernet/ti/icssg/icssg_classifier.c | 1 + drivers/net/ethernet/ti/icssg/icssg_config.c | 6 +- drivers/net/ethernet/ti/icssg/icssg_prueth.c | 122 +++++++++++++++++- drivers/net/ethernet/ti/icssg/icssg_prueth.h | 4 + 4 files changed, 127 insertions(+), 6 deletions(-)