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

Pali Rohár
2021-11-05T21:29:17Z

On Friday 05 November 2021 14:19:46 Roman Bacik wrote:
> Hi Pali,
>
> On Fri, Nov 5, 2021 at 2:09 PM Pali Rohár <pali@kernel.org> 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).
> >
> > Could you provide output of 'lspci -nn -vv' from linux for this card?
> > Or 'pci display.b ?.?.? 0 0x1000' dump from U-Boot?
> > This could help me to under what kind of register that 0x98 is.
> >
> > I can write this part of code, no problem, just I need to see layout of
> > config space of that card.
>
> Here it is:
>
> u-boot> pci display.b ?.?.? 0 1000

Hello! '?.?.?' is placeholder for bus-device-function PCI address of
card. So should replace it by correct address at which is that card.
(What you have sent is dump of PCIe Root Port, probably from 0.0.0)