Re: [PATCH v2 0/2] PCI endpoint test: Add support for capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 21, 2024 at 04:23:19PM +0100, Niklas Cassel wrote:
> Hello all,
> 
> The pci-epf-test driver recently moved to the pci_epc_mem_map() API.
> This API call handle unaligned addresses seamlessly, if the EPC driver
> being used has implemented the .align_addr callback.
> 
> This means that pci-epf-test no longer need any special padding to the
> buffers that is allocated on the host side. (This was only done in order
> to satisfy the EPC's alignment requirements.)
> 
> In fact, to test that the pci_epc_mem_map() API is working as intended,
> it is important that the host side does not only provide buffers that
> are nicely aligned.
> 

I don't agree with the idea of testing the endpoint internal API behavior with
the pci_endpoint_test driver. The driver is supposed to test the functionality
of the endpoint, which it already does. By adding these kind of checks, we are
just going to make the driver bloat.

I suppose if the API behavior is wrong, then it should be caught in the existing
READ/WRITE tests, no?

> However, since not all EPC drivers have implemented the .align_addr
> callback, add support for capabilities in pci-epf-test, and if the
> EPC driver implements the .align_addr callback, set a new
> CAP_UNALIGNED_ACCESS capability. If CAP_UNALIGNED_ACCESS is set, do not
> allocate overly sized buffers on the host side.
> 

This also feels wrong to me. The host driver should care about the alignment
restrictions of the endpoint and then allocate the buffers accordingly, not the
other way.

That being said, I really like to get rid of the hardcoded
'pci_endpoint_test_data::alignment' field and make the driver learn about it
dynamically. I'm just thinking out loud here.

- Mani
 
-- 
மணிவண்ணன் சதாசிவம்




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux