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.
Kind regards,
Niklas
