[PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Jolly Shah]

From: Tejas Patel <tejas.patel@xilinx.com>

Zynqmp firmware driver requires firmware to be present in system.
Zynqmp firmware driver will crash if firmware is not present in system.
For example single arch QEMU, may not have firmware, with such setup
Linux booting fails.

So make zynqmp_firmware driver as optional to disable it if user don't
have firmware in system.

Tejas Patel (2):
  include: linux: firmware: Correct config dependency of zynqmp_eemi_ops
  arch: arm64: xilinx: Make zynqmp_firmware driver optional

 arch/arm64/Kconfig.platforms         | 1 -
 drivers/firmware/xilinx/Kconfig      | 2 ++
 include/linux/firmware/xlnx-zynqmp.h | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.7.4

[PATCH 1/2] include: linux: firmware: Correct config dependency of zynqmp_eemi_ops

[Jolly Shah]

From: Tejas Patel <tejas.patel@xilinx.com>

zynqmp_eemi_ops will be compiled only when CONFIG_ZYNQMP_FIRMWARE is
enabled. So check for CONFIG_ZYNQMP_FIRMWARE instead of checking for
CONFIG_ARCH_ZYNQMP.

Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 include/linux/firmware/xlnx-zynqmp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index e41ad9e..a50a30b 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -311,7 +311,7 @@ struct zynqmp_eemi_ops {
 int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
 			u32 arg2, u32 arg3, u32 *ret_payload);
 
-#if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
+#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)
-- 
2.7.4

Re: [PATCH 1/2] include: linux: firmware: Correct config dependency of zynqmp_eemi_ops

[Michal Simek]

On 09. 01. 20 20:06, Jolly Shah wrote:
> From: Tejas Patel <tejas.patel@xilinx.com>
> 
> zynqmp_eemi_ops will be compiled only when CONFIG_ZYNQMP_FIRMWARE is
> enabled. So check for CONFIG_ZYNQMP_FIRMWARE instead of checking for
> CONFIG_ARCH_ZYNQMP.
> 
> Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  include/linux/firmware/xlnx-zynqmp.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index e41ad9e..a50a30b 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -311,7 +311,7 @@ struct zynqmp_eemi_ops {
>  int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
>  			u32 arg2, u32 arg3, u32 *ret_payload);
>  
> -#if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
> +#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)
> 

I think that make sense to fix this dependency no matter if 2/2 is
applied. That's why adding it to my queue.

Thanks,
Michal

[PATCH 2/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Jolly Shah]

From: Tejas Patel <tejas.patel@xilinx.com>

Zynqmp firmware driver requires firmware to be present in system.
Zynqmp firmware driver will crash if firmware is not present in system.
For example single arch QEMU, may not have firmware, with such setup
Linux booting fails.

So make zynqmp_firmware driver as optional to disable it if user don't
have firmware in system.

Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 arch/arm64/Kconfig.platforms    | 1 -
 drivers/firmware/xilinx/Kconfig | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index b2b504e..563c93d 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -301,7 +301,6 @@ config ARCH_ZX
 
 config ARCH_ZYNQMP
 	bool "Xilinx ZynqMP Family"
-	select ZYNQMP_FIRMWARE
 	help
 	  This enables support for Xilinx ZynqMP Family
 
diff --git a/drivers/firmware/xilinx/Kconfig b/drivers/firmware/xilinx/Kconfig
index bd33bbf..9a9bd19 100644
--- a/drivers/firmware/xilinx/Kconfig
+++ b/drivers/firmware/xilinx/Kconfig
@@ -6,6 +6,8 @@ menu "Zynq MPSoC Firmware Drivers"
 
 config ZYNQMP_FIRMWARE
 	bool "Enable Xilinx Zynq MPSoC firmware interface"
+	depends on ARCH_ZYNQMP
+	default y if ARCH_ZYNQMP
 	select MFD_CORE
 	help
 	  Firmware interface driver is used by different
-- 
2.7.4

Re: [PATCH 2/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Michal Simek]

On 09. 01. 20 20:06, Jolly Shah wrote:
> From: Tejas Patel <tejas.patel@xilinx.com>
> 
> Zynqmp firmware driver requires firmware to be present in system.
> Zynqmp firmware driver will crash if firmware is not present in system.
> For example single arch QEMU, may not have firmware, with such setup
> Linux booting fails.


I think that moving it to firmware Kconfig is good solution. What it is
wrong is that description above where I agree with Sudeep.
It means.
1. User should have option to disable zynqmp firmware driver which is
what this patch allows. It means if someone decides to use different
firmware mechanism it can do it directly by simply y/n option.

2. Autodetection of PMUFW presence is another feature which could be
implemented to have this driver enabled but different mechanism can be
used.

3. Doing this because of missing feature in QEMU is IMHO wrong. QEMU
should be fixed and then you don't have any issue if this should be used
or not.

Just a summary. Remove that QEMU example from commit message and talk to
Edgar to fix single QEMU solution to have that regs mapped all the time.

Thanks,
Michal

Re: [PATCH 2/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Jolly Shah]

Hi Michal,

 > ------Original Message------
 > From: Michal Simek <michal.simek@xilinx.com>
 > Sent:  Monday, February 24, 2020 2:43AM
 > To: Jolly Shah <jolly.shah@xilinx.com>, Ard.biesheuvel@linaro.org 
<ard.biesheuvel@linaro.org>, Mingo@kernel.org <mingo@kernel.org>, 'Greg 
Kh' <gregkh@linuxfoundation.org>, Matt@codeblueprint.co.uk 
<matt@codeblueprint.co.uk>, Sudeep.holla@arm.com <sudeep.holla@arm.com>, 
Hkallweit1@gmail.com <hkallweit1@gmail.com>, Keescook@chromium.org 
<keescook@chromium.org>, Dmitry.torokhov@gmail.com 
<dmitry.torokhov@gmail.com>, Michal Simek <michal.simek@xilinx.com>
 > Cc: Rajan Vaja <rajanv@xilinx.com>, 
Linux-arm-kernel@lists.infradead.org 
<linux-arm-kernel@lists.infradead.org>, Linux-kernel@vger.kernel.org 
<linux-kernel@vger.kernel.org>, Tejas Patel <tejas.patel@xilinx.com>
 > Subject: Re: [PATCH 2/2] arch: arm64: xilinx: Make zynqmp_firmware 
driver optional
 >
> On 09. 01. 20 20:06, Jolly Shah wrote:
>> From: Tejas Patel <tejas.patel@xilinx.com>
>>
>> Zynqmp firmware driver requires firmware to be present in system.
>> Zynqmp firmware driver will crash if firmware is not present in system.
>> For example single arch QEMU, may not have firmware, with such setup
>> Linux booting fails.
> 
> 
> I think that moving it to firmware Kconfig is good solution. What it is
> wrong is that description above where I agree with Sudeep.
> It means.
> 1. User should have option to disable zynqmp firmware driver which is
> what this patch allows. It means if someone decides to use different
> firmware mechanism it can do it directly by simply y/n option.
> 
> 2. Autodetection of PMUFW presence is another feature which could be
> implemented to have this driver enabled but different mechanism can be
> used.
> 
> 3. Doing this because of missing feature in QEMU is IMHO wrong. QEMU
> should be fixed and then you don't have any issue if this should be used
> or not.
> 
> Just a summary. Remove that QEMU example from commit message and talk to
> Edgar to fix single QEMU solution to have that regs mapped all the time.

Pushed another patch as suggested. Will sync up with Edgar.

Thanks,
Jolly Shah

> 
> Thanks,
> Michal
> 

Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Sudeep Holla]

On Thu, Jan 09, 2020 at 11:06:02AM -0800, Jolly Shah wrote:
> From: Tejas Patel <tejas.patel@xilinx.com>
>
> Zynqmp firmware driver requires firmware to be present in system.
> Zynqmp firmware driver will crash if firmware is not present in system.
> For example single arch QEMU, may not have firmware, with such setup
> Linux booting fails.
>
> So make zynqmp_firmware driver as optional to disable it if user don't
> have firmware in system.
>

Why can't it be detected runtime ? How do you handle single binary if you
make this compile time option ?

--
Regards,
Sudeep

RE: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Rajan Vaja]

Hi Sudeep,

Thanks for the reviewing patch.

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: 10 January 2020 05:24 PM
> To: Jolly Shah <JOLLYS@xilinx.com>
> Cc: ard.biesheuvel@linaro.org; mingo@kernel.org; gregkh@linuxfoundation.org;
> matt@codeblueprint.co.uk; hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
> <RAJANV@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Sudeep Holla <sudeep.holla@arm.com>; Tejas Patel
> <TEJASP@xilinx.com>
> Subject: Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver
> optional
> 
> EXTERNAL EMAIL
> 
> On Thu, Jan 09, 2020 at 11:06:02AM -0800, Jolly Shah wrote:
> > From: Tejas Patel <tejas.patel@xilinx.com>
> >
> > Zynqmp firmware driver requires firmware to be present in system.
> > Zynqmp firmware driver will crash if firmware is not present in system.
> > For example single arch QEMU, may not have firmware, with such setup
> > Linux booting fails.
> >
> > So make zynqmp_firmware driver as optional to disable it if user don't
> > have firmware in system.
> >
> 
> Why can't it be detected runtime ? How do you handle single binary if you
> make this compile time option ?
[Rajan] There is PMU register which indicates if firmware is present or not, but in case of single arch QEMU that register will not be available so  there is no way to detect if firmware is present or not from Linux.
Linux firmware crashes while arm_smccc_smc() call for firmware, but before this call there is no way  to identify if firmware is present or not. So we are just giving user an option if they want to use it on single arch
Platform they can disable firmware driver.

Thanks,
Rajan
> --
> Regards,
> Sudeep

Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Sudeep Holla]

On Mon, Jan 13, 2020 at 06:46:52AM +0000, Rajan Vaja wrote:
> Hi Sudeep,
> 
> Thanks for the reviewing patch.
> 
> > -----Original Message-----
> > From: Sudeep Holla <sudeep.holla@arm.com>
> > Sent: 10 January 2020 05:24 PM
> > To: Jolly Shah <JOLLYS@xilinx.com>
> > Cc: ard.biesheuvel@linaro.org; mingo@kernel.org; gregkh@linuxfoundation.org;
> > matt@codeblueprint.co.uk; hkallweit1@gmail.com; keescook@chromium.org;
> > dmitry.torokhov@gmail.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
> > <RAJANV@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-
> > kernel@vger.kernel.org; Sudeep Holla <sudeep.holla@arm.com>; Tejas Patel
> > <TEJASP@xilinx.com>
> > Subject: Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver
> > optional
> > 
> > EXTERNAL EMAIL
> > 
> > On Thu, Jan 09, 2020 at 11:06:02AM -0800, Jolly Shah wrote:
> > > From: Tejas Patel <tejas.patel@xilinx.com>
> > >
> > > Zynqmp firmware driver requires firmware to be present in system.
> > > Zynqmp firmware driver will crash if firmware is not present in system.
> > > For example single arch QEMU, may not have firmware, with such setup
> > > Linux booting fails.
> > >
> > > So make zynqmp_firmware driver as optional to disable it if user don't
> > > have firmware in system.
> > >
> > 
> > Why can't it be detected runtime ? How do you handle single binary if you
> > make this compile time option ?
> [Rajan] There is PMU register which indicates if firmware is present or not,
> but in case of single arch QEMU that register will not be available so
> there is no way to detect if firmware is present or not from Linux.

I am still not that convinced yet.

> Linux firmware crashes while arm_smccc_smc() call for firmware, but before
> this call there is no way  to identify if firmware is present or not. So we
> are just giving user an option if they want to use it on single arch

So IIUC this platform used SMC as transport for EEMI communication. And
PSCI will act as bypass and send the command to PMU. If so why can't
platform PSCI implementation send error to OSPM if it is not implemented.

> Platform they can disable firmware driver.
>

Not an option. With this enable, single binary should work fine on both
QEMU and your platform with this EEMI firmware support. You need to find
a way to detect it dynamically.

--
Regards,
Sudeep

Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver optional

[Michal Simek]

On 13. 01. 20 7:46, Rajan Vaja wrote:
> Hi Sudeep,
> 
> Thanks for the reviewing patch.
> 
>> -----Original Message-----
>> From: Sudeep Holla <sudeep.holla@arm.com>
>> Sent: 10 January 2020 05:24 PM
>> To: Jolly Shah <JOLLYS@xilinx.com>
>> Cc: ard.biesheuvel@linaro.org; mingo@kernel.org; gregkh@linuxfoundation.org;
>> matt@codeblueprint.co.uk; hkallweit1@gmail.com; keescook@chromium.org;
>> dmitry.torokhov@gmail.com; Michal Simek <michals@xilinx.com>; Rajan Vaja
>> <RAJANV@xilinx.com>; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; Sudeep Holla <sudeep.holla@arm.com>; Tejas Patel
>> <TEJASP@xilinx.com>
>> Subject: Re: [PATCH 0/2] arch: arm64: xilinx: Make zynqmp_firmware driver
>> optional
>>
>> EXTERNAL EMAIL
>>
>> On Thu, Jan 09, 2020 at 11:06:02AM -0800, Jolly Shah wrote:
>>> From: Tejas Patel <tejas.patel@xilinx.com>
>>>
>>> Zynqmp firmware driver requires firmware to be present in system.
>>> Zynqmp firmware driver will crash if firmware is not present in system.
>>> For example single arch QEMU, may not have firmware, with such setup
>>> Linux booting fails.
>>>
>>> So make zynqmp_firmware driver as optional to disable it if user don't
>>> have firmware in system.
>>>
>>
>> Why can't it be detected runtime ? How do you handle single binary if you
>> make this compile time option ?
> [Rajan] There is PMU register which indicates if firmware is present or not, but in case of single arch QEMU that register will not be available so  there is no way to detect if firmware is present or not from Linux.
> Linux firmware crashes while arm_smccc_smc() call for firmware, but before this call there is no way  to identify if firmware is present or not. So we are just giving user an option if they want to use it on single arch
> Platform they can disable firmware driver.

If this is just for qemu case why not just add this register for single
instance solution.

Edgar: Can we map this register when there is no second instance to PMU MB?

Thanks,
Michal