On 3/20/26 12:16, Niklas Cassel wrote:
On Fri, Mar 20, 2026 at 10:35:20AM +0100, Christian Bruel wrote:
Hello,
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index
55e128ed82f00ae13b6fe9768cdbe56adbe8f9da..34ba06fb53f04e48c1c05f4aae85e6ecd03ef447
100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -61,6 +61,7 @@
#define STATUS_BAR_SUBRANGE_SETUP_FAIL BIT(15)
#define STATUS_BAR_SUBRANGE_CLEAR_SUCCESS BIT(16)
#define STATUS_BAR_SUBRANGE_CLEAR_FAIL BIT(17)
+#define STATUS_BAR_SUBRANGE_SETUP_NOSPC BIT(18)
#define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR 0x0c
#define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR 0x10
@@ -476,8 +477,11 @@ static int pci_endpoint_test_bar_subrange_cmd(struct
pci_endpoint_test *test,
return -ETIMEDOUT;
status = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_STATUS);
- if (status & fail_bit)
+ if (status & fail_bit) {
+ if (status & STATUS_BAR_SUBRANGE_SETUP_NOSPC)
+ return -ENOSPC;
Perhaps this should be something like:
if (command == COMMAND_BAR_SUBRANGE_SETUP && status &
STATUS_BAR_SUBRANGE_SETUP_SKIP)
I'm not sure about replacing STATUS_BAR_SUBRANGE_SETUP_NOSPC by
STATUS_BAR_SUBRANGE_SETUP_SKIP
The selftest will use if (ret == -ENOSPC), so we need to return this
information (bellow). and semantically SKIP does not imply ENOPSC (instead
of the contrary)
as you prefer,
FWIW, I think an even better solution is to introduce a new register,
named e.g. errno in struct pci_epf_test_reg;
That way, we don't need to take a new bit, e.g.:
+#define STATUS_BAR_SUBRANGE_SETUP_NOSPC BIT(18)
For every unique error code a command can return.
We would simply return STATUS_BAR_SUBRANGE_CLEAR_FAIL, and then the host
side driver looks at the errno register to see the specific error code.
This way, all other _FAIL commands could also return a more specific error
in case of failure.
To minimize changes, what about something like:
unsigned int fail_bit = status & fail_bits;
if (fail_bit == STATUS_BAR_SUBRANGE_SETUP_NOSPC)
return -ENOSPC;
if (fail_bit == STATUS_BAR_SUBRANGE_SETUP_FAIL)
return -EIO;
called with
pci_endpoint_test_bar_subrange_cmd(...,
STATUS_BAR_SUBRANGE_SETUP_FAIL | STATUS_BAR_SUBRANGE_SETUP_NOSPC);
replacing the fail_bit parameter with fail_bits.
Sounds okay to me, but I think the idea of adding an errno register is
more extensible.
But it is not consistent with other tests that use the status field to
carry error information. For example, pci_epf_test_read/write/copy, set
the STATUS_SRC/DTS_ADDR_INVALID bit to distinguish these errors from
other -EINVAL.
Introducing a new errno field would result in having two different APIs
for returning failure status.
In light of the STATUS_SRC/DST_ADDR_INVALID usage, I wonder if my
initial proposal to set a status bit along with the FAIL bit might
actually be simpler and more consistent what is already done here.
thank you,
Christian
Kind regards,
Niklas
