| Message ID | 20240812200437.3581990-2-sean.anderson@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: xilinx: axienet: Multicast fixes and improvements | expand |
On Mon, Aug 12, 2024 at 04:04:34PM -0400, Sean Anderson wrote: > If prmiscuous mode is disabled when there are fewer than four multicast > addresses, then it will to be reflected in the hardware. Fix this by > always clearing the promiscuous mode flag even when we program multicast > addresses. > > Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") > Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > --- > > drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > index ca04c298daa2..e664611c29cf 100644 > --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > @@ -451,6 +451,10 @@ static void axienet_set_multicast_list(struct net_device *ndev) > } else if (!netdev_mc_empty(ndev)) { > struct netdev_hw_addr *ha; > > + reg = axienet_ior(lp, XAE_FMI_OFFSET); > + reg &= ~XAE_FMI_PM_MASK; > + axienet_iow(lp, XAE_FMI_OFFSET, reg); > + Hi Sean, I notice that this replicates code in another part of this function. And that is then factored out into common code as part of the last patch of this series. I guess that it is in the wash, but perhaps it would be nicer to factor out the common promisc mode setting code as part of this patch. Otherwise, this LGTM. > i = 0; > netdev_for_each_mc_addr(ha, ndev) { > if (i >= XAE_MULTICAST_CAM_TABLE_NUM) > -- > 2.35.1.1320.gc452695387.dirty > >
On 8/15/24 10:58, Simon Horman wrote: > On Mon, Aug 12, 2024 at 04:04:34PM -0400, Sean Anderson wrote: >> If prmiscuous mode is disabled when there are fewer than four multicast >> addresses, then it will to be reflected in the hardware. Fix this by >> always clearing the promiscuous mode flag even when we program multicast >> addresses. >> >> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> --- >> >> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> index ca04c298daa2..e664611c29cf 100644 >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c >> @@ -451,6 +451,10 @@ static void axienet_set_multicast_list(struct net_device *ndev) >> } else if (!netdev_mc_empty(ndev)) { >> struct netdev_hw_addr *ha; >> >> + reg = axienet_ior(lp, XAE_FMI_OFFSET); >> + reg &= ~XAE_FMI_PM_MASK; >> + axienet_iow(lp, XAE_FMI_OFFSET, reg); >> + > > Hi Sean, > > I notice that this replicates code in another part of this function. > And that is then factored out into common code as part of the last > patch of this series. > > I guess that it is in the wash, but perhaps it would > be nicer to factor out the common promisc mode setting code > as part of this patch. > > Otherwise, this LGTM. I thought about doing that, but it would have required changing the indentation of ~10 lines and I thought it would be easier to review the patch without that noise. --Sean
On Thu, Aug 15, 2024 at 11:14:41AM -0400, Sean Anderson wrote: > On 8/15/24 10:58, Simon Horman wrote: > > On Mon, Aug 12, 2024 at 04:04:34PM -0400, Sean Anderson wrote: > >> If prmiscuous mode is disabled when there are fewer than four multicast > >> addresses, then it will to be reflected in the hardware. Fix this by > >> always clearing the promiscuous mode flag even when we program multicast > >> addresses. > >> > >> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") > >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > >> --- > >> > >> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> index ca04c298daa2..e664611c29cf 100644 > >> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c > >> @@ -451,6 +451,10 @@ static void axienet_set_multicast_list(struct net_device *ndev) > >> } else if (!netdev_mc_empty(ndev)) { > >> struct netdev_hw_addr *ha; > >> > >> + reg = axienet_ior(lp, XAE_FMI_OFFSET); > >> + reg &= ~XAE_FMI_PM_MASK; > >> + axienet_iow(lp, XAE_FMI_OFFSET, reg); > >> + > > > > Hi Sean, > > > > I notice that this replicates code in another part of this function. > > And that is then factored out into common code as part of the last > > patch of this series. > > > > I guess that it is in the wash, but perhaps it would > > be nicer to factor out the common promisc mode setting code > > as part of this patch. > > > > Otherwise, this LGTM. > > I thought about doing that, but it would have required changing the > indentation of ~10 lines and I thought it would be easier to review > the patch without that noise. Sure, I was also wrestling with that in my mind. I think we can stick with what you have :) Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c index ca04c298daa2..e664611c29cf 100644 --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c @@ -451,6 +451,10 @@ static void axienet_set_multicast_list(struct net_device *ndev) } else if (!netdev_mc_empty(ndev)) { struct netdev_hw_addr *ha; + reg = axienet_ior(lp, XAE_FMI_OFFSET); + reg &= ~XAE_FMI_PM_MASK; + axienet_iow(lp, XAE_FMI_OFFSET, reg); + i = 0; netdev_for_each_mc_addr(ha, ndev) { if (i >= XAE_MULTICAST_CAM_TABLE_NUM)
If prmiscuous mode is disabled when there are fewer than four multicast addresses, then it will to be reflected in the hardware. Fix this by always clearing the promiscuous mode flag even when we program multicast addresses. Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver") Signed-off-by: Sean Anderson <sean.anderson@linux.dev> --- drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++++ 1 file changed, 4 insertions(+)