Messages in this thread | | | Subject | Re: [PATCH] drivers: soc: xilinx: fix firmware driver Kconfig dependency | From | Michal Simek <> | Date | Thu, 9 Apr 2020 12:43:34 +0200 |
| |
On 09. 04. 20 11:09, Arnd Bergmann wrote: > On Thu, Apr 9, 2020 at 8:37 AM Michal Simek <michal.simek@xilinx.com> wrote: >> >> On 08. 04. 20 17:52, Arnd Bergmann wrote: >>> The firmware driver is optional, but the power driver depends on it, >>> which needs to be reflected in Kconfig to avoid link errors: >>> >>> aarch64-linux-ld: drivers/soc/xilinx/zynqmp_power.o: in function `zynqmp_pm_isr': >>> zynqmp_power.c:(.text+0x284): undefined reference to `zynqmp_pm_invoke_fn' >>> >>> The firmware driver can probably be allowed for compile-testing as >>> well, so it's best to drop the dependency on the ZYNQ platform >>> here and allow building as long as the firmware code is built-in. >>> >>> Fixes: ab272643d723 ("drivers: soc: xilinx: Add ZynqMP PM driver") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> --- >>> drivers/soc/xilinx/Kconfig | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig >>> index 223f1f9d0922..646512d7276f 100644 >>> --- a/drivers/soc/xilinx/Kconfig >>> +++ b/drivers/soc/xilinx/Kconfig >>> @@ -19,7 +19,7 @@ config XILINX_VCU >>> >>> config ZYNQMP_POWER >>> bool "Enable Xilinx Zynq MPSoC Power Management driver" >>> - depends on PM && ARCH_ZYNQMP >>> + depends on PM && ZYNQMP_FIRMWARE >>> default y >>> select MAILBOX >>> select ZYNQMP_IPI_MBOX >>> @@ -35,7 +35,7 @@ config ZYNQMP_POWER >>> config ZYNQMP_PM_DOMAINS >>> bool "Enable Zynq MPSoC generic PM domains" >>> default y >>> - depends on PM && ARCH_ZYNQMP && ZYNQMP_FIRMWARE >>> + depends on PM && ZYNQMP_FIRMWARE >>> select PM_GENERIC_DOMAINS >>> help >>> Say yes to enable device power management through PM domains >>> >> >> The same issue is likely with others drivers dependencies too which >> depends on ARCH_ZYNQMP. >> >> It means all drivers which includes "linux/firmware/xlnx-zynqmp.h" and >> call zynqmp_pm_get_eemi_ops() should depend on ZYNQMP_FIRMWARE instead >> of ARCH_ZYNQMP. > > The only one I see that has a hard dependency on ARCH_ZYNQMP > without allowing compile-testing at the moment is drivers/edac/synopsys_edac.c > but that doesn't use the firmware interface. > > What I see in the header are declarations for exported functions: > > int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1, > u32 arg2, u32 arg3, u32 *ret_payload); > #if IS_REACHABLE(CONFIG_ZYNQMP_FIRMWARE) > const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void); > #else > static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void) > { > return ERR_PTR(-ENODEV); > } > #endif > > The second one already allows compile-testing by turning into an > inline stub, but zynqmp_pm_invoke_fn() does not, and this is the > one causing the problem here. > > I still think my patch is a good fix for that issue, but if you want to > handle both interfaces the same way, we can also do that, either > removing the stub and using a proper dependency, or using > the same stub trick for both.
I have really not a problem with your fix above because the patch which was applied has started to remove dependencies on ARCH_ZYNQMP. It shouldn't be there because the same interface is used for new Xilinx Versal device.
That header has been reworked by patches from here. (last one) http://lkml.kernel.org/r/20200318115452.GA2491827@kroah.com that's why changes has to go on the top of it.
Anyway feel free to take it directly or I will take it and send you pull request. But will also look at other dependencies to make sure that they are correct.
Acked-by: Michal Simek <michal.simek@xilinx.com>
Thanks, Michal
| |