Project Home
Project Home
Documents
Documents
Wiki
Wiki
Discussion Forums
Discussions
Project Information
Project Info
BroadcastCommunity.qnx.com will be offline from May 31 6:00pm until June 2 12:00AM for upcoming system upgrades. For more information please go to https://community.qnx.com/sf/discussion/do/listPosts/projects.bazaar/discussion.bazaar.topc28418
Forum Topic - Concerns with offset parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions: (3 Items)
   
Concerns with offset parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions  
I have a public open-source project that aims to porting of Linux CAN-bus drivers to QNX.
Repository: https://github.com/Deniz-Eren/dev-can-linux

I have question regarding the offset input parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of 
functions.

Looking at QNX library file: qnx710/target/qnx7/usr/include/pci/pci.h
We see comment: "The following functions provide read access to the device dependent portion of the 256/4096 byte PCI/
PCIe configuration space from 0x40 to 0xFF/0xFFF."

This sounds like the offset input parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions means 
offset=0x00 refers to the first byte of the configuration space that starts at 0x40 and goes through to to 0xFF/0xFFF. 
However, in practice one of our community members found that when they were testing with the peak_pci.c driver, that 
these pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions do not start at 0x40 but start at 0x00. So to 
access the configuration space at 0x40 to 0xFF/0xFFF we had to add 0x40 to the address offset to get equivalent 
behaviour as that of the same code running on Linux.

This peak_pci.c, is one of the drivers we have ported from Linux and it's the Peak driver: https://github.com/Deniz-Eren
/dev-can-linux/blob/main/src/kernel/drivers/net/can/sja1000/peak_pci.c

At line 590 you see:
    err = pci_read_config_word(pdev, 0x2e, &sub_sys_id);
    if (err)
            goto failure_release_regions;

At line 597 you see:
    err = pci_write_config_word(pdev, 0x44, 0);
    if (err)
            goto failure_release_regions;

This peak_pci.c have been taken from the Linux kernel source code as-is (https://github.com/torvalds/linux). To 
interface to QNX we have implemented the Linux functions pci_write_config_word() and pci_read_config_word() as https://
github.com/Deniz-Eren/dev-can-linux/blob/main/src/pci.c

As you can see, in our implementation we had to call pci_device_cfg_rd*() and pci_device_cfg_wr*() with a +0x40 for the 
peak_pci.c driver to work properly.

This means, either the comment block for pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions in file 
qnx710/target/qnx7/usr/include/pci/pci.h is misleading or incorrect and it should say 0x00 to 0xFF/0xFFF. Otherwise, the
 implementation of these functions are incorrect and they need to be updated to actually give access to 0x40 to 0xFF/
0xFFF.

Any comments?
Re: Concerns with offset parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions  
The offset is absolute and starts at 0. These functions will return an
error if the offset is below 0x40.

I agree the comment can be made clearer.

--Elad

On Tue, 2023-10-17 at 02:07 -0400, Deniz Eren wrote:
> I have a public open-source project that aims to porting of Linux
> CAN-bus drivers to QNX.
> Repository: https://github.com/Deniz-Eren/dev-can-linux
> 
> I have question regarding the offset input parameter of
> pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions.
> 
> Looking at QNX library file: qnx710/target/qnx7/usr/include/pci/pci.h
> We see comment: "The following functions provide read access to the
> device dependent portion of the 256/4096 byte PCI/PCIe configuration
> space from 0x40 to 0xFF/0xFFF."
> 
> This sounds like the offset input parameter of pci_device_cfg_rd*()
> and pci_device_cfg_wr*() series of functions means offset=0x00 refers
> to the first byte of the configuration space that starts at 0x40 and
> goes through to to 0xFF/0xFFF. However, in practice one of our
> community members found that when they were testing with the
> peak_pci.c driver, that these pci_device_cfg_rd*() and
> pci_device_cfg_wr*() series of functions do not start at 0x40 but
> start at 0x00. So to access the configuration space at 0x40 to
> 0xFF/0xFFF we had to add 0x40 to the address offset to get equivalent
> behaviour as that of the same code running on Linux.
> 
> This peak_pci.c, is one of the drivers we have ported from Linux and
> it's the Peak driver:
> https://github.com/Deniz-Eren/dev-can-linux/blob/main/src/kernel/drivers/net/can/sja1000/peak_pci.c
> 
> At line 590 you see:
>     err = pci_read_config_word(pdev, 0x2e, &sub_sys_id);
>     if (err)
>             goto failure_release_regions;
> 
> At line 597 you see:
>     err = pci_write_config_word(pdev, 0x44, 0);
>     if (err)
>             goto failure_release_regions;
> 
> This peak_pci.c have been taken from the Linux kernel source code as-
> is (https://github.com/torvalds/linux). To interface to QNX we have
> implemented the Linux functions pci_write_config_word() and
> pci_read_config_word() as
> https://github.com/Deniz-Eren/dev-can-linux/blob/main/src/pci.c
> 
> As you can see, in our implementation we had to call
> pci_device_cfg_rd*() and pci_device_cfg_wr*() with a +0x40 for the
> peak_pci.c driver to work properly.
> 
> This means, either the comment block for pci_device_cfg_rd*() and
> pci_device_cfg_wr*() series of functions in file
> qnx710/target/qnx7/usr/include/pci/pci.h is misleading or incorrect
> and it should say 0x00 to 0xFF/0xFFF. Otherwise, the implementation
> of these functions are incorrect and they need to be updated to
> actually give access to 0x40 to 0xFF/0xFFF.
> 
> Any comments?
> 
> 
> 
> _______________________________________________
> 
> OSTech
> http://community.qnx.com/sf/go/post122397
> To cancel your subscription to this discussion, please e-mail
> ostech-core_os-unsubscribe@community.qnx.com

Re: Concerns with offset parameter of pci_device_cfg_rd*() and pci_device_cfg_wr*() series of functions  
Thank you Elad.

At least this gives us the confidence our implementation has done the correct thing.