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
