On 2022-01-10 3:53 a.m., Jonathan Gray wrote:
> On Mon, Jan 10, 2022 at 09:17:25PM +1100, Jonathan Gray wrote:
>> 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

Ok, then still missing from the document is the semantic distinctions
between the return types and the correct usage for those types with
pci_mapreg_info and pci_mapreg_map.

There is also the undocumented PCI_MAPREG_TYPE and PCI_MAPREG_MEM_TYPE
macros too:

This usage should be incorrect?
if (type == PCI_MAPREG_TYPE_MEM)

I think correct form should be then
if (PCI_MAPREG_TYPE(type) == PCI_MAPREG_TYPE_MEM)
    switch (PCI_MAPREG_MEM_TYPE(type))
    PCI_MAPREG_MEM_TYPE_32BIT_1M:
    PCI_MAPREG_MEM_TYPE_32BIT:
        pci_mapreg_map for 32 bit bar
    PCI_MAPREG_MEM_TYPE_64BIT:
        pci_mapreg_map for 64 bit bar

This pci stack doesn't also seem to handle pci devices with a 16 bit
bar, but I also don't know if such devices actually exist so maybe this
doesn't matter

> expanding on this some more
> 
> 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 10:51:36 -0000
> @@ -128,6 +128,11 @@ pointers.
>  attempts to determine if the BAR referenced by
>  .Fa reg
>  describes a valid register mapping.
> +If it does
> +.Fa typep
> +will point to a value with flags matching those described for the return 
> value
> +of
> +.Nm pci_mapreg_type .
>  .Pp
>  .Nm pci_mapreg_type
>  returns the type of register access for the registers at the BAR
> @@ -135,18 +140,22 @@ 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 describes a valid mapping, 0 if not.
> +.Pp
>  .Nm pci_mapreg_type
> -returns either
> -.Dv PCI_MAPREG_TYPE_IO
> -or
> -.Dv PCI_MAPREG_TYPE_MEM .
> +returns a value with flags for type information.
> +.Dv PCI_MAPREG_TYPE_IO ,
> +.Dv PCI_MAPREG_TYPE_MEM ,
> +.Dv PCI_MAPREG_MEM_TYPE_32BIT ,
> +.Dv PCI_MAPREG_MEM_TYPE_32BIT_1M and
> +.Dv PCI_MAPREG_MEM_TYPE_64BIT .
>  .Sh SEE ALSO
>  .Xr pci 4 ,
>  .Xr bus_space 9 ,

-- 
Ted Bullock <tbull...@comlore.com>

Reply via email to