[PATCH v6 1/2] net: brcm: netXtreme driver

[Pali Rohár]

On Friday 05 November 2021 22:09:47 Pali Rohár wrote:
> On Friday 05 November 2021 12:54:24 Roman Bacik wrote:
> > > > +	pci_read_word16(bp->pdev,
> > > > +			PCI_SUBSYSTEM_VENDOR_ID,
> > > > +			&bp->subsystem_vendor);
> > > > +	pci_read_word16(bp->pdev, PCI_SUBSYSTEM_ID, &bp-
> > > >subsystem_device);
> > > > +	pci_read_word16(bp->pdev, PCI_COMMAND, &bp->cmd_reg);
> > > > +	pci_read_byte(bp->pdev, PCICFG_ME_REGISTER, &bp->pf_num);
> > >
> > > PCICFG_ME_REGISTER looks like an error as there is no such PCI config
> > > space macro. What you are trying to read into pf_num? Currently I do not
> > > know what "pf" abbreviation could mean.
> > 
> > PF stands for physical function and pf_num is the number of physical
> > functions configured.
> > The macro is defined in bnxt.h:
> > #define PCICFG_ME_REGISTER                      0x98
> 
> pci_read_byte() reads from PCI(e) config space, which is standardized.
> Therefore only standard macro constants from include/pci.h should be
> used. Standard PCI config header is 64 byte long and after that is
> linked list of capabilities. Order of capabilities is not defined.
> Extended capabilities from linked list should be located by macro
> constants PCI_CAP_ID_*.
> 
> So above register is part of some extended capability. Correctly it
> should be used some function to locate starting offset of that extended
> capability based on PCI_CAP_ID_* (see pci.h file for these functions)
> and then access that register as offset + PCI_* constant (which defined
> as relative to the start of extended capability). In case standard macro
> for this constant in pci.h is missing, it is a good idea to define it,
> or copy it from linux header file pci_regs.h (to have consistent naming
> of macros).

Just one example how to read PCIe Link Control Register (to make it
clear what I mean):

int ret;
u16 lnkctl;
int pci_exp_off;
pci_exp_off = dm_pci_find_capability(dev, PCI_CAP_ID_EXP);
if (!pci_exp_off) return -ENOENT;
ret = dm_pci_read_config16(dev, pci_exp_off + PCI_EXP_LNKCTL, &lnkctl);
if (ret) return ret;