Re: [PATCH v3] platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

Bjorn Helgaas
2023-12-27T00:35:00Z

On Mon, Dec 25, 2023 at 06:26:56PM +0900, Shin'ichiro Kawasaki wrote:
> ...

> +static int p2sb_valid_resource(struct resource *res)
> +{
> + return res->flags ? 0 : -ENOENT;
> +}

This got worse because it's *named* like a boolean, but the return
value can't be used like a boolean, which makes callers really hard to
read, e.g., this:

if (p2sb_valid_resource(res))
/* do something */

does exactly the opposite of what the reader expects.

I see that you want to use this -ENOENT return value in the callers:

> +static int p2sb_scan_and_cache(struct pci_bus *bus, unsigned int devfn)
> +{
> + ...
> + return p2sb_valid_resource(&p2sb_resources[PCI_FUNC(devfn)].res);
> +}

> + * 0 on success or appropriate errno value on error.
> + */
> +int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem)
> +{
> + ...
> + ret = p2sb_valid_resource(&cache->res);
> + if (ret)
> + return ret;

But I think these would be much clearer as something like this:

static bool p2sb_valid_resource(struct resource *res)
{
if (res->flags)
return true;

return false;
}

static int p2sb_scan_and_cache(struct pci_bus *bus, unsigned int devfn)
{
...
if (!p2sb_valid_resource(&p2sb_resources[PCI_FUNC(devfn)].res))
return -ENOENT;

return 0;
}

Bjorn