Re: [PATCH] pci/controller/pcie-rcar-host: Hold the reference returned by of_find_matching_node

[Bjorn Helgaas <helgaas@xxxxxxxxxx>]

On Tue, Jun 21, 2022 at 03:01:45PM +0800, Liang He wrote:
> In rcar_pcie_init(), we need to hold the reference returned by
> of_find_matching_node() which is used to call of_node_put() for
> refcount balance.
> 
> Signed-off-by: Liang He <windhl@xxxxxxx>
> ---
>  drivers/pci/controller/pcie-rcar-host.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
> index 997c4df6a1e7..405ec3d64f30 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -1158,7 +1158,10 @@ static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst
>  
>  static int __init rcar_pcie_init(void)
>  {
> -	if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) {
> +	struct device_node *np = of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match);
> +
> +	if (np) {
> +		of_node_put(np);

I think this is correct, but it would be nicer to update the way this
driver uses of_device_get_match_data(), e.g.,

  struct rcar_variant {
    int (*phy_init_fn)(struct rcar_pcie_host *host);
    bool hook_abort;
  };

  struct rcar_pcie_host {
    ...
    const struct rcar_variant *variant;
  };

  static int rcar_pcie_probe(...)
  {
    host->variant = of_device_get_match_data(dev);
    err = host->variant->phy_init_fn(host);
    ...

  #ifdef CONFIG_ARM
    if (host->variant->hook_abort) {
  #ifdef CONFIG_ARM_LPAE
      hook_fault_code(17, ...);
  # else
      hook_fault_code(22, ...);
  #endif
    }
  #endif
  }

Or keep the hook in a separate function called from rcar_pcie_probe()
if you think that's cleaner.

I'm not sure hook_fault_code() needs to be called separately as a
device_initcall().  The pci-ixp4xx.c driver does it in
ixp4xx_pci_probe(), so I assume rcar could do it in probe as well.

>  #ifdef CONFIG_ARM_LPAE
>  		hook_fault_code(17, rcar_pcie_aarch32_abort_handler, SIGBUS, 0,
>  				"asynchronous external abort");
> -- 
> 2.25.1
>