Re: [PATCH 3/3] misc: pci_endpoint_test: Handle -ENOSPC in subrange mapping test case

[Niklas Cassel <cassel@kernel.org>]

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