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

[Bjorn Helgaas <helgaas@kernel.org>]

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