On Thu, Oct 17, 2019 at 3:26 PM Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx> wrote: > > /* Register PCI driver */ > > - pcireg_rc = pci_register_driver(&safexcel_pci_driver); > > -#endif > > + ret = pci_register_driver(&safexcel_pci_driver); > > > > -#if IS_ENABLED(CONFIG_OF) > > /* Register platform driver */ > > - ofreg_rc = platform_driver_register(&crypto_safexcel); > > - #if IS_ENABLED(CONFIG_PCI) > > - /* Return success if either PCI or OF registered OK */ > > - return pcireg_rc ? ofreg_rc : 0; > > - #else > > - return ofreg_rc; > > - #endif > > -#else > > - #if IS_ENABLED(CONFIG_PCI) > > - return pcireg_rc; > > - #else > > - return -EINVAL; > > - #endif > > -#endif > > + if (IS_ENABLED(CONFIG_OF) && !ret) { > > > Hmm ... this would make it skip the OF registration if the PCIE > registration failed. Note that the typical embedded system will > have a PCIE subsystem (e.g. Marvell A7K/A8K does) but will have > the EIP embedded on the SoC as an OF device. > > So the question is: is it possible somehow that PCIE registration > fails while OF registration does pass? Because in that case, this > code would be wrong ... I don't see how it would fail. When CONFIG_PCI is disabled, pci_register_driver() does nothing, and the pci_driver as well as everything referenced from it will be silently dropped from the object file. If CONFIG_PCI is enabled, then the driver will be registered to the PCI subsystem, waiting for a device to show up, but the driver registration does not care about whether there is such a device. > Other than that, I don't care much how this code is implemented > as long as it works for both my use cases, being an OF embedded > device (on a SoC _with_ or _without_ PCIE support) and a device > on a PCIE board in a PCI (which has both PCIE and OF support). Yes, that should be fine. There are a lot of drivers that support multiple bus interfaces, and this is the normal way to handle them. Arnd