On Mon, 11 Sep 2023 15:14:56 +0300 Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote: > Use FIELD_GET() to extract PCIe Negotiated Link Width field instead of > custom masking and shifting. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > --- > drivers/net/ethernet/intel/igb/e1000_mac.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/e1000_mac.c b/drivers/net/ethernet/intel/igb/e1000_mac.c > index caf91c6f52b4..5a23b9cfec6c 100644 > --- a/drivers/net/ethernet/intel/igb/e1000_mac.c > +++ b/drivers/net/ethernet/intel/igb/e1000_mac.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright(c) 2007 - 2018 Intel Corporation. */ > > +#include <linux/bitfield.h> > #include <linux/if_ether.h> > #include <linux/delay.h> > #include <linux/pci.h> > @@ -50,9 +51,8 @@ s32 igb_get_bus_info_pcie(struct e1000_hw *hw) > break; > } > > - bus->width = (enum e1000_bus_width)((pcie_link_status & > - PCI_EXP_LNKSTA_NLW) >> > - PCI_EXP_LNKSTA_NLW_SHIFT); > + bus->width = (enum e1000_bus_width)FIELD_GET(PCI_EXP_LNKSTA_NLW, > + pcie_link_status); This cast is a bit ugly given it takes the values 0, 1, 2, 3 and we extra a field that the spec says contains 1, 2, 4, 8 etc Hence it only works because only 1 and 2 are used I think... Not nice. Also, whilst looking at this I note that e1000e has it's own defines for PCIE_LINK_WIDTH_MASK and PCIE_LINK_WIDTH_SHIFT Looks like those should be changed to use the standard defines. For extra giggles there are two e1000_bus_width enum definitions in different headers. Actual patch is good - just 'interesting' stuff noticed whilst looking at it :) Jonathan > } > > reg = rd32(E1000_STATUS);
