| Message ID | 20200408155224.2070880-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | d0384eedcde21276ac51f57c641f875605024b32 |
| Headers | show |
| Series | drivers: soc: xilinx: fix firmware driver Kconfig dependency | expand |
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. Thanks, Michal
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. Arnd
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
On 09. 04. 20 12:43, Michal Simek wrote: > 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> Applied to zynqmp/soc. Thanks, Michal
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 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(-)