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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bjorn,

On Wed, Jun 22, 2022 at 12:57 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> 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>

> > --- 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().

Yes it doesn, as hook_fault_code() is __init.

> The pci-ixp4xx.c driver does it in
> ixp4xx_pci_probe(), so I assume rcar could do it in probe as well.

The pci-ixp4xx.c driver uses builtin_platform_driver_probe(), and
ixp4xx_pci_probe() is marked __init.


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux