On Mon, Jan 10, 2022 at 02:24:05AM -0700, Ted Bullock wrote:
>
> On 2022-01-10 2:18 a.m., Jonathan Gray wrote:
> > On Sat, Jan 08, 2022 at 01:43:32PM -0700, Ted Bullock wrote:
> > > The manpage incorrectly describes the behaviour and usage of
> > > pci_mapreg_probe(9). This function does not return 0 for success and !0
> > > for failure as described in the manual, see the diff below for a
> > > possible re-wording and corrected description.
> >
> > I agree the return value is wrong but your description introduces an
> > error in that type bits can contain PCI_MAPREG_MEM_TYPE_64BIT for example.
> > Which is something that the pci_mapreg_type text also gets wrong.
> >
> > How about this to start with?
>
> It certainly corrects the mechanical issue with the returns, it doesn't help
> though with understanding how to use the function correctly though. Under
> what circumstances will the type bits get set as you indicate? I wasn't at
> all clear about that from peering through the call tree.
as the define would suggest, when it is a 64-bit mem bar
for example
0x0010: BAR mem 64bit addr: 0x00000000f1220000/0x00010000
xhci0 at pci0 dev 20 function 0 "Intel 9 Series xHCI" rev 0x03: bar 0x10 type
0x4 : msi, xHCI 1.0
Index: xhci_pci.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/xhci_pci.c,v
retrieving revision 1.10
diff -u -p -r1.10 xhci_pci.c
--- xhci_pci.c 18 Nov 2018 08:46:57 -0000 1.10
+++ xhci_pci.c 10 Jan 2022 09:54:09 -0000
@@ -134,6 +134,7 @@ xhci_pci_attach(struct device *parent, s
pci_intr_handle_t ih;
pcireg_t reg;
int error;
+ pcireg_t type;
reg = pci_mapreg_type(pa->pa_pc, pa->pa_tag, PCI_CBMEM);
if (pci_mapreg_map(pa, PCI_CBMEM, reg, 0, &psc->sc.iot, &psc->sc.ioh,
@@ -141,6 +142,9 @@ xhci_pci_attach(struct device *parent, s
printf(": can't map mem space\n");
return;
}
+
+ if (pci_mapreg_probe(pa->pa_pc, pa->pa_tag, 0x10, &type))
+ printf(": bar 0x10 type 0x%x ", type);
psc->sc_pc = pa->pa_pc;
psc->sc_tag = pa->pa_tag;
from pcireg.h:
#define PCI_MAPREG_TYPE(mr) \
((mr) & PCI_MAPREG_TYPE_MASK)
#define PCI_MAPREG_TYPE_MASK 0x00000001
#define PCI_MAPREG_TYPE_MEM 0x00000000
#define PCI_MAPREG_TYPE_IO 0x00000001
#define PCI_MAPREG_MEM_TYPE(mr) \
((mr) & PCI_MAPREG_MEM_TYPE_MASK)
#define PCI_MAPREG_MEM_TYPE_MASK 0x00000006
#define PCI_MAPREG_MEM_TYPE_32BIT 0x00000000
#define PCI_MAPREG_MEM_TYPE_32BIT_1M 0x00000002
#define PCI_MAPREG_MEM_TYPE_64BIT 0x00000004
#define _PCI_MAPREG_TYPEBITS(reg) \
(PCI_MAPREG_TYPE(reg) == PCI_MAPREG_TYPE_IO ? \
reg & PCI_MAPREG_TYPE_MASK : \
reg & (PCI_MAPREG_TYPE_MASK|PCI_MAPREG_MEM_TYPE_MASK))
>
> Anyway it's certainly a weakness of the document. This at least stops the
> error from the return values.
>
> >
> > Index: pci_mapreg_map.9
> > ===================================================================
> > RCS file: /cvs/src/share/man/man9/pci_mapreg_map.9,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 pci_mapreg_map.9
> > --- pci_mapreg_map.9 23 Feb 2019 04:54:25 -0000 1.1
> > +++ pci_mapreg_map.9 10 Jan 2022 09:13:30 -0000
> > @@ -135,12 +135,14 @@ referenced by
> > .Fa reg .
> > .Sh RETURN VALUES
> > .Nm pci_mapreg_map ,
> > -.Nm pci_mapreg_info ,
> > and
> > -.Nm pci_mapreg_probe
> > +.Nm pci_mapreg_info
> > return 0 on success, or an
> > .Xr errno 2
> > style value on failure.
> > +.Pp
> > +.Nm pci_mapreg_probe
> > +returns 1 if if the BAR is implemented, 0 if not.
> > .Pp
> > .Nm pci_mapreg_type
> > returns either
>
> --
> Ted Bullock <tbull...@comlore.com>
>
>
