Re: [PATCH 2/6] firmware: xilinx: add macros of node ids for error event

Re: [PATCH 2/6] firmware: xilinx: add macros of node ids for error event

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> Add macros for the Node-Id of Error events.
> 
> Move supported api callback ids from zynqmp-power to zynqmp-firmware.
> 
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  drivers/soc/xilinx/zynqmp_power.c    |  6 ------
>  include/linux/firmware/xlnx-zynqmp.h | 13 +++++++++++++
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> index c556623..76478fe 100644
> --- a/drivers/soc/xilinx/zynqmp_power.c
> +++ b/drivers/soc/xilinx/zynqmp_power.c
> @@ -46,12 +46,6 @@ static const char *const suspend_modes[] = {
>  
>  static enum pm_suspend_mode suspend_mode = PM_SUSPEND_MODE_STD;
>  
> -enum pm_api_cb_id {
> -	PM_INIT_SUSPEND_CB = 30,
> -	PM_ACKNOWLEDGE_CB,
> -	PM_NOTIFY_CB,
> -};
> -
>  static void zynqmp_pm_get_callback_data(u32 *buf)
>  {
>  	zynqmp_pm_invoke_fn(GET_CALLBACK_DATA, 0, 0, 0, 0, buf);
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 6557832..c715756 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -60,6 +60,19 @@
>  #define XILINX_ZYNQMP_PM_FPGA_FULL	0x0U
>  #define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
>  
> +/*
> + * Node IDs for the Error Events.
> + */
> +#define EVENT_ERROR_PMC_ERR1	(0x28100000U)
> +#define EVENT_ERROR_PMC_ERR2	(0x28104000U)
> +#define EVENT_ERROR_PSM_ERR1	(0x28108000U)
> +#define EVENT_ERROR_PSM_ERR2	(0x2810C000U)
> +
> +enum pm_api_cb_id {
> +	PM_INIT_SUSPEND_CB = 30,

No information about why PM_ACKNOWLEDGE_CB was removed. Doing it via
separate patch would be the best.

M

> +	PM_NOTIFY_CB = 32,
> +};
> +
>  enum pm_api_id {
>  	PM_GET_API_VERSION = 1,
>  	PM_REGISTER_NOTIFIER = 5,
> 

Re: [PATCH 1/6] firmware: xilinx: add register notifier in zynqmp firmware

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> In zynqmp-firmware, register notifier is not supported, add support of
> register notifier in zynqmp-firmware.
> 
> Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp-debug.c |  2 +-
>  drivers/firmware/xilinx/zynqmp.c       | 23 +++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h   | 11 ++++++++++-
>  3 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp-debug.c b/drivers/firmware/xilinx/zynqmp-debug.c
> index 99606b3..7841cb7 100644
> --- a/drivers/firmware/xilinx/zynqmp-debug.c
> +++ b/drivers/firmware/xilinx/zynqmp-debug.c
> @@ -2,7 +2,7 @@
>  /*
>   * Xilinx Zynq MPSoC Firmware layer for debugfs APIs
>   *
> - *  Copyright (C) 2014-2018 Xilinx, Inc.
> + *  Copyright (C) 2014-2021 Xilinx, Inc.

no reason for this change when you don't change this file.

>   *
>   *  Michal Simek <michal.simek@xilinx.com>
>   *  Davorin Mista <davorin.mista@aggios.com>
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 15b13832..33f190e 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -1037,6 +1037,29 @@ int zynqmp_pm_aes_engine(const u64 address, u32 *out)
>  EXPORT_SYMBOL_GPL(zynqmp_pm_aes_engine);
>  
>  /**
> + * zynqmp_pm_register_notifier() - PM API for register a subsystem
> + *                                to be notified about specific
> + *                                event/error.
> + * @node:	Node ID to which the event is related.
> + * @event:	Event Mask of Error events for which wants to get notified.
> + * @wake:	Wake subsystem upon capturing the event if value 1
> + * @enable:	Enable the registration for value 1, disable for value 0
> + *
> + * This function is used to register/un-register for particular node-event
> + * combination in firmware.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +
> +int zynqmp_pm_register_notifier(const u32 node, const u32 event,
> +				const u32 wake, const u32 enable)
> +{
> +	return zynqmp_pm_invoke_fn(PM_REGISTER_NOTIFIER, node, event,
> +				   wake, enable, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_register_notifier);
> +
> +/**
>   * zynqmp_pm_system_shutdown - PM call to request a system shutdown or restart
>   * @type:	Shutdown or restart? 0 for shutdown, 1 for restart
>   * @subtype:	Specifies which system should be restarted or shut down
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 9d1a5c1..6557832 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -2,7 +2,7 @@
>  /*
>   * Xilinx Zynq MPSoC Firmware layer
>   *
> - *  Copyright (C) 2014-2019 Xilinx
> + *  Copyright (C) 2014-2021 Xilinx
>   *
>   *  Michal Simek <michal.simek@xilinx.com>
>   *  Davorin Mista <davorin.mista@aggios.com>
> @@ -62,6 +62,7 @@
>  
>  enum pm_api_id {
>  	PM_GET_API_VERSION = 1,
> +	PM_REGISTER_NOTIFIER = 5,
>  	PM_SYSTEM_SHUTDOWN = 12,
>  	PM_REQUEST_NODE = 13,
>  	PM_RELEASE_NODE = 14,
> @@ -411,6 +412,8 @@ int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
>  				 u32 *value);
>  int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
>  				 u32 value);
> +int zynqmp_pm_register_notifier(const u32 node, const u32 event,
> +				const u32 wake, const u32 enable);
>  #else
>  static inline int zynqmp_pm_get_api_version(u32 *version)
>  {
> @@ -622,6 +625,12 @@ static inline int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int zynqmp_pm_register_notifier(const u32 node, const u32 event,
> +					      const u32 wake, const u32 enable)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  #endif /* __FIRMWARE_ZYNQMP_H__ */
> 

M

Re: [PATCH 3/6] firmware: xilinx: export the feature check of zynqmp firmware

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Export the zynqmp_pm_feature(), so it can be use by other as to get API
> version available in firmware.
> 
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp.c     | 3 ++-
>  include/linux/firmware/xlnx-zynqmp.h | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 33f190e..ce16a72 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -153,7 +153,7 @@ static noinline int do_fw_call_hvc(u64 arg0, u64 arg1, u64 arg2,
>   *
>   * Return: Returns status, either success or error+reason
>   */
> -static int zynqmp_pm_feature(u32 api_id)
> +int zynqmp_pm_feature(const u32 api_id)
>  {
>  	int ret;
>  	u32 ret_payload[PAYLOAD_ARG_CNT];
> @@ -190,6 +190,7 @@ static int zynqmp_pm_feature(u32 api_id)
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(zynqmp_pm_feature);
>  
>  /**
>   * zynqmp_pm_invoke_fn() - Invoke the system-level platform management layer
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index c715756..06ea5a4 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -427,6 +427,7 @@ int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
>  				 u32 value);
>  int zynqmp_pm_register_notifier(const u32 node, const u32 event,
>  				const u32 wake, const u32 enable);
> +int zynqmp_pm_feature(const u32 api_id);
>  #else
>  static inline int zynqmp_pm_get_api_version(u32 *version)
>  {
> @@ -644,6 +645,11 @@ static inline int zynqmp_pm_register_notifier(const u32 node, const u32 event,
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int zynqmp_pm_feature(const u32 api_id)
> +{
> +	return -ENODEV;
> +}
>  #endif
>  
>  #endif /* __FIRMWARE_ZYNQMP_H__ */
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [PATCH 5/6] firmware: xilinx: instantiate xilinx event manager driver

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> Register simple platform device to instantiate Xilinx event
> manager driver.
> 
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index ce16a72..31f7857 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -23,6 +23,7 @@
>  #include <linux/hashtable.h>
>  
>  #include <linux/firmware/xlnx-zynqmp.h>
> +#include <linux/firmware/xlnx-event-manager.h>
>  #include "zynqmp-debug.h"
>  
>  /* Max HashMap Order for PM API feature check (1<<7 = 128) */
> @@ -31,6 +32,8 @@
>  static bool feature_check_enabled;
>  static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
>  
> +static struct platform_device *em_dev;
> +
>  /**
>   * struct pm_api_feature_data - PM API Feature data
>   * @pm_api_id:		PM API Id, used as key to index into hashmap
> @@ -1412,6 +1415,15 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
>  
>  	zynqmp_pm_api_debugfs_init();
>  
> +	np = of_find_compatible_node(NULL, NULL, "xlnx,versal");
> +	if (np) {
> +		em_dev = platform_device_register_data(&pdev->dev, "xlnx_event_manager",
> +						       -1, NULL, 0);
> +		if (IS_ERR(em_dev))
> +			dev_err_probe(&pdev->dev, PTR_ERR(pdev), "EM register fail with error\n");
> +	}
> +	of_node_put(np);
> +
>  	return of_platform_populate(dev->of_node, NULL, NULL, dev);
>  }
>  
> @@ -1429,6 +1441,8 @@ static int zynqmp_firmware_remove(struct platform_device *pdev)
>  		kfree(feature_data);
>  	}
>  
> +	platform_device_unregister(em_dev);
> +
>  	return 0;
>  }
>  
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [PATCH 4/6] drivers: soc: xilinx: add xilinx event management driver

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> Xilinx event management driver provide an interface to subscribe
> or unsubscribe for the event/callback supported by firmware. Agent can use
> this driver to register for Error Event, Device Event and Suspend callback.
> This driver only allow one agent per event to do registration. Driver will
> return an error in case of multiple registration for the same event.
> 
> This driver gets notification from firmware through ATF as SGI. During
> initialization, event manager driver register handler for SGI used for
> notification. It also provide SGI number info to ATF by using
> IOCTL_REGISTER_SGI call to ATF.
> 
> After receiving notification from firmware, driver makes SMC call to ATF
> to get IPI data. From the IPI data provided by ATF, event manager
> identified cause of event and forward that event/callback notification to
> respective subscribed driver. After this, in case of Error Event, driver
> performs unregistration as firmware expecting from agent to do
> re-registration if agent wants to get notified on second occurrence of
> error event.
> 
> Add new IOCTL id IOCTL_REGISTER_SGI = 25 which use to register SGI on ATF.

nit: Instead of ATF you should use TF-A

> 
> Older firmware doesn't have all required support for event handling
> which is required by event manager driver. So add check for register
> notifier version in event manager driver.
> 
> Xilinx event management driver provide support to subscribe for multiple
> error events with the use of Event Mask in single call of
> xlnx_register_event(). Agent driver can provide 'Event' parameter value as
> ORed of multiple event masks to register single callback for multiple
> events. For example, to register callback for event=0x1 and event=0x2 for
> the given node, agent can provide event=0x3 (0x1 | 0x2). It is not
> possible to register multiple event for different nodes in single
> registration call.
> 
> Also provide support to receive multiple error events as in single
> notification from firmware and then forward it to subscribed drivers via
> registered callback one by one.
> 
> Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  MAINTAINERS                                 |   6 +
>  drivers/soc/xilinx/Kconfig                  |  10 +
>  drivers/soc/xilinx/Makefile                 |   1 +
>  drivers/soc/xilinx/xlnx_event_manager.c     | 598 ++++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-event-manager.h |  36 ++
>  include/linux/firmware/xlnx-zynqmp.h        |   2 +
>  6 files changed, 653 insertions(+)
>  create mode 100644 drivers/soc/xilinx/xlnx_event_manager.c
>  create mode 100644 include/linux/firmware/xlnx-event-manager.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3a2384e..6299069 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20096,6 +20096,12 @@ T:	git https://github.com/Xilinx/linux-xlnx.git
>  F:	Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
>  F:	drivers/phy/xilinx/phy-zynqmp.c
>  
> +XILINX EVENT MANAGEMENT DRIVER
> +M:	Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> +S:	Maintained
> +F:	drivers/soc/xilinx/xlnx_event_manager.c
> +F:	include/linux/firmware/xlnx-event-manager.h
> +
>  XILLYBUS DRIVER
>  M:	Eli Billauer <eli.billauer@gmail.com>
>  L:	linux-kernel@vger.kernel.org
> diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> index 53af911..8a755a5 100644
> --- a/drivers/soc/xilinx/Kconfig
> +++ b/drivers/soc/xilinx/Kconfig
> @@ -25,4 +25,14 @@ config ZYNQMP_PM_DOMAINS
>  	  Say yes to enable device power management through PM domains
>  	  If in doubt, say N.
>  
> +config XLNX_EVENT_MANAGER
> +	bool "Enable Xilinx Event Management Driver"
> +	depends on ZYNQMP_FIRMWARE
> +	default ZYNQMP_FIRMWARE
> +	help
> +	  Say yes to enable event management support for Xilinx.
> +	  This driver uses firmware driver as an interface for event/power
> +	  management request to firmware.
> +
> +	  If in doubt, say N.
>  endmenu
> diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
> index 9854e6f..41e585b 100644
> --- a/drivers/soc/xilinx/Makefile
> +++ b/drivers/soc/xilinx/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_ZYNQMP_POWER)	+= zynqmp_power.o
>  obj-$(CONFIG_ZYNQMP_PM_DOMAINS) += zynqmp_pm_domains.o
> +obj-$(CONFIG_XLNX_EVENT_MANAGER)	+= xlnx_event_manager.o
> diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c
> new file mode 100644
> index 0000000..bf726ee
> --- /dev/null
> +++ b/drivers/soc/xilinx/xlnx_event_manager.c
> @@ -0,0 +1,598 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx Event Management Driver
> + *
> + *  Copyright (C) 2021 Xilinx, Inc.
> + *
> + *  Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> + */
> +
> +#include <linux/cpuhotplug.h>
> +#include <linux/firmware/xlnx-event-manager.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +#include <linux/hashtable.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number1);
> +
> +static int virq_sgi;
> +static int event_manager_availability = -EACCES;
> +
> +/* SGI number used for Event management driver */
> +#define XLNX_EVENT_SGI_NUM	(15)
> +
> +/* Max number of driver can register for same event */
> +#define MAX_DRIVER_PER_EVENT	(10U)
> +
> +/* Max HashMap Order for PM API feature check (1<<7 = 128) */
> +#define REGISTERED_DRIVER_MAX_ORDER	(7)
> +
> +#define MAX_BITS	(32U) /* Number of bits available for error mask */
> +
> +#define FIRMWARE_VERSION_MASK			(0xFFFFU)
> +#define REGISTER_NOTIFIER_FIRMWARE_VERSION	(2U)
> +
> +static DEFINE_HASHTABLE(reg_driver_map, REGISTERED_DRIVER_MAX_ORDER);
> +static int sgi_num = XLNX_EVENT_SGI_NUM;

make this module parameter with default on XLNX_EVENT_SGI_NUM.

> +
> +/**
> + * struct registered_event_data - Registered Event Data.
> + * @key:		key is the combine id(Node-Id | Event-Id) of type u64
> + *			where upper u32 for Node-Id and lower u32 for Event-Id,
> + *			And this used as key to index into hashmap.
> + * @agent_data:		Data passed back to handler function.
> + * @cb_type:		Type of Api callback, like PM_NOTIFY_CB, etc.
> + * @eve_cb:		Function pointer to store the callback function.
> + * @wake:		If this flag set, firmware will wakeup processor if is
> + *			in sleep or power down state.
> + * @hentry:		hlist_node that hooks this entry into hashtable.
> + */
> +struct registered_event_data {
> +	u64 key;
> +	enum pm_api_cb_id cb_type;
> +	void *agent_data;
> +
> +	event_cb_func_t eve_cb;
> +	bool wake;
> +	struct hlist_node hentry;
> +};
> +
> +static bool xlnx_is_error_event(const u32 node_id)
> +{
> +	if (node_id == EVENT_ERROR_PMC_ERR1 ||
> +	    node_id == EVENT_ERROR_PMC_ERR2 ||
> +	    node_id == EVENT_ERROR_PSM_ERR1 ||
> +	    node_id == EVENT_ERROR_PSM_ERR2)
> +		return true;
> +
> +	return false;
> +}
> +
> +static int xlnx_add_cb_for_notify_event(const u32 node_id, const u32 event, const bool wake,
> +					event_cb_func_t cb_fun,	void *data)
> +{
> +	u64 key = 0;
> +	struct registered_event_data *eve_data;
> +
> +	key = ((u64)node_id << 32U) | (u64)event;
> +	/* Check for existing entry in hash table for given key id */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> +		if (eve_data->key == key) {

Don't you need to check eve_data here too?

> +			pr_err("Found as already registered\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* Add new entry if not present */
> +	eve_data = kmalloc(sizeof(*eve_data), GFP_KERNEL);
> +	if (!eve_data)
> +		return -ENOMEM;
> +
> +	eve_data->key = key;
> +	eve_data->cb_type = PM_NOTIFY_CB;
> +	eve_data->eve_cb = cb_fun;
> +	eve_data->wake = wake;
> +	eve_data->agent_data = data;
> +
> +	hash_add(reg_driver_map, &eve_data->hentry, key);
> +
> +	return 0;
> +}
> +
> +static int xlnx_add_cb_for_suspend(event_cb_func_t cb_fun, void *data)
> +{
> +	struct registered_event_data *eve_data;
> +
> +	/* Check for existing entry in hash table for given cb_type */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, PM_INIT_SUSPEND_CB) {
> +		if (eve_data->cb_type == PM_INIT_SUSPEND_CB) {

Same as above to make sure that eve_data is not NULL.
The same is also below.

> +			pr_err("Found as already registered\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* Add new entry if not present */
> +	eve_data = kmalloc(sizeof(*eve_data), GFP_KERNEL);
> +	if (!eve_data)
> +		return -ENOMEM;
> +
> +	eve_data->key = 0;
> +	eve_data->cb_type = PM_INIT_SUSPEND_CB;
> +	eve_data->eve_cb = cb_fun;
> +	eve_data->agent_data = data;
> +
> +	hash_add(reg_driver_map, &eve_data->hentry, PM_INIT_SUSPEND_CB);
> +
> +	return 0;
> +}
> +
> +static int xlnx_remove_cb_for_suspend(event_cb_func_t cb_fun)
> +{
> +	bool is_callback_found = false;
> +	struct registered_event_data *eve_data;
> +
> +	/* Check for existing entry in hash table for given cb_type */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, PM_INIT_SUSPEND_CB) {
> +		if (eve_data->cb_type == PM_INIT_SUSPEND_CB &&
> +		    eve_data->eve_cb == cb_fun) {
> +			is_callback_found = true;
> +			/* remove an object from a hashtable */
> +			hash_del(&eve_data->hentry);
> +			kfree(eve_data);
> +		}
> +	}
> +	if (!is_callback_found) {
> +		pr_warn("Didn't find any registered callback for suspend event\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int xlnx_remove_cb_for_notify_event(const u32 node_id, const u32 event,
> +					   event_cb_func_t cb_fun)
> +{
> +	bool is_callback_found = false;
> +	struct registered_event_data *eve_data;
> +	u64 key = ((u64)node_id << 32U) | (u64)event;
> +
> +	/* Check for existing entry in hash table for given key id */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> +		if (eve_data->key == key &&
> +		    eve_data->eve_cb == cb_fun) {
> +			is_callback_found = true;
> +			/* remove an object from a hashtable */
> +			hash_del(&eve_data->hentry);
> +			kfree(eve_data);
> +		}
> +	}
> +	if (!is_callback_found) {
> +		pr_warn("Didn't find any registered callback for 0x%x 0x%x\n",
> +			node_id, event);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * xlnx_register_event() - Register for the event.
> + * @cb_type:	Type of callback from pm_api_cb_id,
> + *			PM_NOTIFY_CB - for Error Events,
> + *			PM_INIT_SUSPEND_CB - for suspend callback.
> + * @node_id:	Node-Id related to event.
> + * @event:	Event Mask for the Error Event.
> + * @wake:	Flag specifying whether the subsystem should be woken upon
> + *		event notification.
> + * @cb_fun:	Function pointer to store the callback function.
> + * @data:	Pointer for the driver instance.
> + *
> + * Return:	Returns 0 on successful registration else error code.
> + */
> +int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32 node_id, const u32 event,
> +			const bool wake, event_cb_func_t cb_fun, void *data)
> +{
> +	int ret = 0;
> +	u32 eve;
> +	int pos;
> +
> +	if (event_manager_availability)
> +		return event_manager_availability;
> +
> +	if (cb_type != PM_NOTIFY_CB && cb_type != PM_INIT_SUSPEND_CB) {
> +		pr_err("%s() Unsupported Callback 0x%x\n", __func__, cb_type);
> +		return -EINVAL;
> +	}
> +
> +	if (!cb_fun)
> +		return -EFAULT;
> +
> +	if (cb_type == PM_INIT_SUSPEND_CB) {
> +		ret = xlnx_add_cb_for_suspend(cb_fun, data);
> +	} else {
> +		if (!xlnx_is_error_event(node_id)) {
> +			/* Add entry for Node-Id/Event in hash table */
> +			ret = xlnx_add_cb_for_notify_event(node_id, event, wake, cb_fun, data);
> +		} else {
> +			/* Add into Hash table */
> +			for (pos = 0; pos < MAX_BITS; pos++) {
> +				eve = event & (1 << pos);
> +				if (!eve)
> +					continue;
> +
> +				/* Add entry for Node-Id/Eve in hash table */
> +				ret = xlnx_add_cb_for_notify_event(node_id, eve, wake, cb_fun,
> +								   data);
> +				/* Break the loop if got error */
> +				if (ret)
> +					break;
> +			}
> +			if (ret) {
> +				/* Skip the Event for which got the error */
> +				pos--;
> +				/* Remove registered(during this call) event from hash table */
> +				for ( ; pos >= 0; pos--) {
> +					eve = event & (1 << pos);
> +					if (!eve)
> +						continue;
> +					xlnx_remove_cb_for_notify_event(node_id, eve, cb_fun);
> +				}
> +			}
> +		}
> +
> +		if (ret) {
> +			pr_err("%s() failed for 0x%x and 0x%x: %d\r\n", __func__, node_id,
> +			       event, ret);
> +			return ret;
> +		}
> +
> +		/* Register for Node-Id/Event combination in firmware */
> +		ret = zynqmp_pm_register_notifier(node_id, event, wake, true);
> +		if (ret) {
> +			pr_err("%s() failed for 0x%x and 0x%x: %d\r\n", __func__, node_id,
> +			       event, ret);
> +			/* Remove already registered event from hash table */
> +			if (xlnx_is_error_event(node_id)) {
> +				for (pos = 0; pos < MAX_BITS; pos++) {
> +					eve = event & (1 << pos);
> +					if (!eve)
> +						continue;
> +					xlnx_remove_cb_for_notify_event(node_id, eve, cb_fun);
> +				}
> +			} else {
> +				xlnx_remove_cb_for_notify_event(node_id, event, cb_fun);
> +			}
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(xlnx_register_event);
> +
> +/**
> + * xlnx_unregister_event() - Unregister for the event.
> + * @cb_type:	Type of callback from pm_api_cb_id,
> + *			PM_NOTIFY_CB - for Error Events,
> + *			PM_INIT_SUSPEND_CB - for suspend callback.
> + * @node_id:	Node-Id related to event.
> + * @event:	Event Mask for the Error Event.
> + * @cb_fun:	Function pointer of callback function.
> + *
> + * Return:	Returns 0 on successful unregistration else error code.
> + */
> +int xlnx_unregister_event(const enum pm_api_cb_id cb_type, const u32 node_id, const u32 event,
> +			  event_cb_func_t cb_fun)
> +{
> +	int ret;
> +	u32 eve, pos;
> +
> +	if (event_manager_availability)
> +		return event_manager_availability;
> +
> +	if (cb_type != PM_NOTIFY_CB && cb_type != PM_INIT_SUSPEND_CB) {
> +		pr_err("%s() Unsupported Callback 0x%x\n", __func__, cb_type);
> +		return -EINVAL;
> +	}
> +
> +	if (!cb_fun)
> +		return -EFAULT;
> +
> +	if (cb_type == PM_INIT_SUSPEND_CB) {
> +		ret = xlnx_remove_cb_for_suspend(cb_fun);
> +	} else {
> +		/* Remove Node-Id/Event from hash table */
> +		if (!xlnx_is_error_event(node_id)) {
> +			xlnx_remove_cb_for_notify_event(node_id, event, cb_fun);
> +		} else {
> +			for (pos = 0; pos < MAX_BITS; pos++) {
> +				eve = event & (1 << pos);
> +				if (!eve)
> +					continue;
> +
> +				xlnx_remove_cb_for_notify_event(node_id, eve, cb_fun);
> +			}
> +		}
> +
> +		/* Un-register for Node-Id/Event combination */
> +		ret = zynqmp_pm_register_notifier(node_id, event, false, false);
> +		if (ret) {
> +			pr_err("%s() failed for 0x%x and 0x%x: %d\n",
> +			       __func__, node_id, event, ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(xlnx_unregister_event);
> +
> +static void xlnx_call_suspend_cb_handler(const u32 *payload)
> +{
> +	bool is_callback_found = false;
> +	struct registered_event_data *eve_data;
> +	u32 cb_type = payload[0];
> +
> +	/* Check for existing entry in hash table for given cb_type */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, cb_type) {
> +		if (eve_data->cb_type == cb_type) {
> +			eve_data->eve_cb(&payload[0], eve_data->agent_data);
> +			is_callback_found = true;
> +		}
> +	}
> +	if (!is_callback_found)
> +		pr_warn("Didn't find any registered callback for suspend event\n");
> +}
> +
> +static void xlnx_call_notify_cb_handler(const u32 *payload)
> +{
> +	bool is_callback_found = false;
> +	struct registered_event_data *eve_data;
> +	u64 key = ((u64)payload[1] << 32U) | (u64)payload[2];
> +	int ret;
> +
> +	/* Check for existing entry in hash table for given key id */
> +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> +		if (eve_data->key == key) {
> +			eve_data->eve_cb(&payload[0], eve_data->agent_data);
> +			is_callback_found = true;
> +
> +			/* re register with firmware to get future events */
> +			ret = zynqmp_pm_register_notifier(payload[1], payload[2],
> +							  eve_data->wake, true);
> +			if (ret) {
> +				pr_err("%s() failed for 0x%x and 0x%x: %d\r\n", __func__,
> +				       payload[1], payload[2], ret);
> +				/* Remove already registered event from hash table */
> +				xlnx_remove_cb_for_notify_event(payload[1], payload[2],
> +								eve_data->eve_cb);
> +			}
> +		}
> +	}
> +	if (!is_callback_found)
> +		pr_warn("Didn't find any registered callback for 0x%x 0x%x\n",
> +			payload[1], payload[2]);
> +}
> +
> +static void xlnx_get_event_callback_data(u32 *buf)
> +{
> +	zynqmp_pm_invoke_fn(GET_CALLBACK_DATA, 0, 0, 0, 0, buf);
> +}
> +
> +static irqreturn_t xlnx_event_handler(int irq, void *dev_id)
> +{
> +	u32 cb_type, node_id, event, pos;
> +	u32 payload[CB_MAX_PAYLOAD_SIZE] = {0};
> +	u32 event_data[CB_MAX_PAYLOAD_SIZE] = {0};
> +
> +	/* Get event data */
> +	xlnx_get_event_callback_data(payload);
> +
> +	/* First element is callback type, others are callback arguments */
> +	cb_type = payload[0];
> +
> +	if (cb_type == PM_NOTIFY_CB) {
> +		node_id = payload[1];
> +		event = payload[2];
> +		if (!xlnx_is_error_event(node_id)) {
> +			xlnx_call_notify_cb_handler(payload);
> +		} else {
> +			/*
> +			 * Each call back function expecting payload as an input arguments.
> +			 * We can get multiple error events as in one call back through error
> +			 * mask. So payload[2] may can contain multiple error events.
> +			 * In reg_driver_map database we store data in the combination of single
> +			 * node_id-error combination.
> +			 * So coping the payload message into event_data and update the
> +			 * event_data[2] with Error Mask for single error event and use
> +			 * event_data as input argument for registered call back function.
> +			 *
> +			 */
> +			memcpy(event_data, payload, (4 * CB_MAX_PAYLOAD_SIZE));
> +			/* Support Multiple Error Event */
> +			for (pos = 0; pos < MAX_BITS; pos++) {
> +				if ((0 == (event & (1 << pos))))
> +					continue;
> +				event_data[2] = (event & (1 << pos));
> +				xlnx_call_notify_cb_handler(event_data);
> +			}
> +		}
> +	} else if (cb_type == PM_INIT_SUSPEND_CB) {
> +		xlnx_call_suspend_cb_handler(payload);
> +	} else {
> +		pr_err("%s() Unsupported Callback %d\n", __func__, cb_type);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int xlnx_event_cpuhp_start(unsigned int cpu)
> +{
> +	enable_percpu_irq(virq_sgi, IRQ_TYPE_NONE);
> +
> +	return 0;
> +}
> +
> +static int xlnx_event_cpuhp_down(unsigned int cpu)
> +{
> +	disable_percpu_irq(virq_sgi);
> +
> +	return 0;
> +}
> +
> +static void xlnx_disable_percpu_irq(void *data)
> +{
> +	disable_percpu_irq(virq_sgi);
> +}
> +
> +static int xlnx_event_init_sgi(struct platform_device *pdev)
> +{
> +	int ret = 0;
> +	int cpu = smp_processor_id();
> +	/*
> +	 * IRQ related structures are used for the following:
> +	 * for each SGI interrupt ensure its mapped by GIC IRQ domain
> +	 * and that each corresponding linux IRQ for the HW IRQ has
> +	 * a handler for when receiving an interrupt from the remote
> +	 * processor.
> +	 */
> +	struct irq_domain *domain;
> +	struct irq_fwspec sgi_fwspec;
> +	struct device_node *interrupt_parent = NULL;
> +	struct device *parent = pdev->dev.parent;
> +
> +	/* Find GIC controller to map SGIs. */
> +	interrupt_parent = of_irq_find_parent(parent->of_node);
> +	if (!interrupt_parent) {
> +		dev_err(&pdev->dev, "Failed to find property for Interrupt parent\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Each SGI needs to be associated with GIC's IRQ domain. */
> +	domain = irq_find_host(interrupt_parent);
> +	of_node_put(interrupt_parent);
> +
> +	/* Each mapping needs GIC domain when finding IRQ mapping. */
> +	sgi_fwspec.fwnode = domain->fwnode;
> +
> +	/*
> +	 * When irq domain looks at mapping each arg is as follows:
> +	 * 3 args for: interrupt type (SGI), interrupt # (set later), type
> +	 */
> +	sgi_fwspec.param_count = 1;
> +
> +	/* Set SGI's hwirq */
> +	sgi_fwspec.param[0] = sgi_num;
> +	virq_sgi = irq_create_fwspec_mapping(&sgi_fwspec);
> +
> +	per_cpu(cpu_number1, cpu) = cpu;
> +	ret = request_percpu_irq(virq_sgi, xlnx_event_handler, "xlnx_event_mgmt",
> +				 &cpu_number1);
> +	WARN_ON(ret);
> +	if (ret) {
> +		irq_dispose_mapping(virq_sgi);
> +		return ret;
> +	}
> +
> +	irq_to_desc(virq_sgi);
> +	irq_set_status_flags(virq_sgi, IRQ_PER_CPU);
> +
> +	return ret;
> +}
> +
> +static void xlnx_event_cleanup_sgi(struct platform_device *pdev)
> +{
> +	int cpu = smp_processor_id();
> +
> +	per_cpu(cpu_number1, cpu) = cpu;
> +
> +	cpuhp_remove_state(CPUHP_AP_ONLINE_DYN);
> +
> +	on_each_cpu(xlnx_disable_percpu_irq, NULL, 1);
> +
> +	irq_clear_status_flags(virq_sgi, IRQ_PER_CPU);
> +	free_percpu_irq(virq_sgi, &cpu_number1);
> +	irq_dispose_mapping(virq_sgi);
> +}
> +
> +static int xlnx_event_manager_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	ret = zynqmp_pm_feature(PM_REGISTER_NOTIFIER);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Feature check failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	if ((ret & FIRMWARE_VERSION_MASK) <
> +	    REGISTER_NOTIFIER_FIRMWARE_VERSION) {
> +		dev_err(&pdev->dev, "Register notifier version error. Expected Firmware: v%d - Found: v%d\n",
> +			REGISTER_NOTIFIER_FIRMWARE_VERSION,
> +			ret & FIRMWARE_VERSION_MASK);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/* Initialize the SGI */
> +	ret = xlnx_event_init_sgi(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "SGI Init has been failed with %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Setup function for the CPU hot-plug cases */
> +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/event:starting",
> +			  xlnx_event_cpuhp_start, xlnx_event_cpuhp_down);
> +
> +	ret = zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_REGISTER_SGI, sgi_num,
> +				  0, NULL);
> +	if (ret) {
> +		dev_err(&pdev->dev, "SGI Registration over ATF failed with %d\n", ret);
> +		xlnx_event_cleanup_sgi(pdev);
> +		return ret;
> +	}
> +
> +	event_manager_availability = 0;
> +
> +	dev_info(&pdev->dev, "Xilinx Event Management driver probed\n");
> +
> +	return ret;
> +}
> +
> +static int xlnx_event_manager_remove(struct platform_device *pdev)
> +{
> +	int i;
> +	struct registered_event_data *eve_data;
> +	struct hlist_node *tmp;
> +	int ret;
> +
> +	hash_for_each_safe(reg_driver_map, i, tmp, eve_data, hentry) {
> +		hash_del(&eve_data->hentry);
> +		kfree(eve_data);
> +	}
> +
> +	ret = zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_REGISTER_SGI, 0, 1, NULL);
> +	if (ret)
> +		dev_err(&pdev->dev, "SGI unregistration over ATF failed with %d\n", ret);
> +
> +	xlnx_event_cleanup_sgi(pdev);
> +
> +	event_manager_availability = -EACCES;
> +
> +	return ret;
> +}
> +
> +static struct platform_driver xlnx_event_manager_driver = {
> +	.probe = xlnx_event_manager_probe,
> +	.remove = xlnx_event_manager_remove,
> +	.driver = {
> +		.name = "xlnx_event_manager",
> +	},
> +};
> +module_platform_driver(xlnx_event_manager_driver);
> diff --git a/include/linux/firmware/xlnx-event-manager.h b/include/linux/firmware/xlnx-event-manager.h
> new file mode 100644
> index 0000000..09758ab
> --- /dev/null
> +++ b/include/linux/firmware/xlnx-event-manager.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _FIRMWARE_XLNX_EVENT_MANAGER_H_
> +#define _FIRMWARE_XLNX_EVENT_MANAGER_H_
> +
> +#include <linux/firmware/xlnx-zynqmp.h>
> +
> +#define CB_MAX_PAYLOAD_SIZE	(4U) /*In payload maximum 32bytes */
> +
> +/************************** Exported Function *****************************/
> +
> +typedef void (*event_cb_func_t)(const u32 *payload, void *data);
> +
> +#if IS_REACHABLE(CONFIG_XLNX_EVENT_MANAGER)
> +int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32 node_id,
> +			const u32 event, const bool wake,
> +			event_cb_func_t cb_fun, void *data);
> +
> +int xlnx_unregister_event(const enum pm_api_cb_id cb_type, const u32 node_id,
> +			  const u32 event, event_cb_func_t cb_fun);
> +#else
> +static inline int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32 node_id,
> +				      const u32 event, const bool wake,
> +				      event_cb_func_t cb_fun, void *data)
> +{
> +	return -ENODEV;
> +}
> +
> +istatic inline int xlnx_unregister_event(const enum pm_api_cb_id cb_type, const u32 node_id,
> +					 const u32 event, event_cb_func_t cb_fun)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
> +#endif /* _FIRMWARE_XLNX_EVENT_MANAGER_H_ */
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 06ea5a4..f3e857c 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -133,6 +133,8 @@ enum pm_ioctl_id {
>  	IOCTL_READ_PGGS = 15,
>  	/* Set healthy bit value */
>  	IOCTL_SET_BOOT_HEALTH_STATUS = 17,
> +	/* Register SGI to ATF */
> +	IOCTL_REGISTER_SGI = 25,
>  };
>  
>  enum pm_query_id {
> 

The rest looks good to me. Please push interface to TFA first to make
sure that API won't change.

Thanks,
Michal

Re: [PATCH 5/6] firmware: xilinx: instantiate xilinx event manager driver

[Michal Simek]

On 6/21/21 1:48 PM, Michal Simek wrote:
> 
> 
> On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
>> Register simple platform device to instantiate Xilinx event
>> manager driver.
>>
>> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
>> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
>> ---
>>  drivers/firmware/xilinx/zynqmp.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
>> index ce16a72..31f7857 100644
>> --- a/drivers/firmware/xilinx/zynqmp.c
>> +++ b/drivers/firmware/xilinx/zynqmp.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/hashtable.h>
>>  
>>  #include <linux/firmware/xlnx-zynqmp.h>
>> +#include <linux/firmware/xlnx-event-manager.h>
>>  #include "zynqmp-debug.h"
>>  
>>  /* Max HashMap Order for PM API feature check (1<<7 = 128) */
>> @@ -31,6 +32,8 @@
>>  static bool feature_check_enabled;
>>  static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
>>  
>> +static struct platform_device *em_dev;
>> +
>>  /**
>>   * struct pm_api_feature_data - PM API Feature data
>>   * @pm_api_id:		PM API Id, used as key to index into hashmap
>> @@ -1412,6 +1415,15 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
>>  
>>  	zynqmp_pm_api_debugfs_init();
>>  
>> +	np = of_find_compatible_node(NULL, NULL, "xlnx,versal");
>> +	if (np) {
>> +		em_dev = platform_device_register_data(&pdev->dev, "xlnx_event_manager",
>> +						       -1, NULL, 0);
>> +		if (IS_ERR(em_dev))
>> +			dev_err_probe(&pdev->dev, PTR_ERR(pdev), "EM register fail with error\n");

Above you have IS_ERR(em_dev) and here PTR_ERR(pdev) this should be
PTR_ERR(em_dev).


>> +	}
>> +	of_node_put(np);
>> +
>>  	return of_platform_populate(dev->of_node, NULL, NULL, dev);
>>  }
>>  
>> @@ -1429,6 +1441,8 @@ static int zynqmp_firmware_remove(struct platform_device *pdev)
>>  		kfree(feature_data);
>>  	}
>>  
>> +	platform_device_unregister(em_dev);
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> Acked-by: Michal Simek <michal.simek@xilinx.com>

With above fixed please add this line.

Thanks,
Michal

Re: [PATCH 6/6] driver: soc: xilinx: register for power events in zynqmp power driver

[Michal Simek]

On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> With Xilinx Event Management driver, all types of events like power and
> error gets handled from single place as part of event management driver.
> 
> So power events(SUSPEND_POWER_REQUEST and SUSPEND_SYSTEM_SHUTDOWN)
> also gets handled by event management driver instead of zynqmp_power
> driver.
> 
> zynqmp-power driver use event management driver and provide callback
> function for Suspend and shutdown handler, which will be called by event
> management driver when respective event is arrived.
> 
> If event management driver is not available than use ipi-mailbox rx channel
> or IPI interrupt IRQ handler for power events (suspend/shutdown) same as
> current zynqmp-power driver.
> 
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> ---
>  drivers/soc/xilinx/zynqmp_power.c | 48 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> index 76478fe..fe7be17 100644
> --- a/drivers/soc/xilinx/zynqmp_power.c
> +++ b/drivers/soc/xilinx/zynqmp_power.c
> @@ -16,6 +16,7 @@
>  #include <linux/suspend.h>
>  
>  #include <linux/firmware/xlnx-zynqmp.h>
> +#include <linux/firmware/xlnx-event-manager.h>
>  #include <linux/mailbox/zynqmp-ipi-message.h>
>  
>  /**
> @@ -30,6 +31,7 @@ struct zynqmp_pm_work_struct {
>  
>  static struct zynqmp_pm_work_struct *zynqmp_pm_init_suspend_work;
>  static struct mbox_chan *rx_chan;
> +static bool event_registered;
>  
>  enum pm_suspend_mode {
>  	PM_SUSPEND_MODE_FIRST = 0,
> @@ -51,6 +53,19 @@ static void zynqmp_pm_get_callback_data(u32 *buf)
>  	zynqmp_pm_invoke_fn(GET_CALLBACK_DATA, 0, 0, 0, 0, buf);
>  }
>  
> +static void suspend_event_callback(const u32 *payload, void *data)
> +{
> +	/* First element is callback API ID, others are callback arguments */
> +	if (work_pending(&zynqmp_pm_init_suspend_work->callback_work))
> +		return;
> +
> +	/* Copy callback arguments into work's structure */
> +	memcpy(zynqmp_pm_init_suspend_work->args, &payload[1],
> +	       sizeof(zynqmp_pm_init_suspend_work->args));
> +
> +	queue_work(system_unbound_wq, &zynqmp_pm_init_suspend_work->callback_work);
> +}
> +
>  static irqreturn_t zynqmp_pm_isr(int irq, void *data)
>  {
>  	u32 payload[CB_PAYLOAD_SIZE];
> @@ -179,7 +194,32 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>  	if (pm_api_version < ZYNQMP_PM_VERSION)
>  		return -ENODEV;
>  
> -	if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
> +	/*
> +	 * First try to use Xilinx Event Manager by registering suspend_event_callback
> +	 * for suspend/shutdown event.
> +	 * If xlnx_register_event() returns -EACCES (Xilinx Event Manager
> +	 * is not available to use) or -ENODEV(Xilinx Event Manager not compiled),
> +	 * then use ipi-mailbox or interrupt method.
> +	 */
> +	ret = xlnx_register_event(PM_INIT_SUSPEND_CB, 0, 0, false,
> +				  suspend_event_callback, NULL);
> +	if (!ret) {
> +		zynqmp_pm_init_suspend_work = devm_kzalloc(&pdev->dev,
> +							   sizeof(struct zynqmp_pm_work_struct),
> +							   GFP_KERNEL);
> +		if (!zynqmp_pm_init_suspend_work) {
> +			xlnx_unregister_event(PM_INIT_SUSPEND_CB, 0, 0,
> +					      suspend_event_callback);
> +			return -ENOMEM;
> +		}
> +		event_registered = true;
> +
> +		INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
> +			  zynqmp_pm_init_suspend_work_fn);
> +	} else if (ret != -EACCES && ret != -ENODEV) {
> +		dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
> +		return ret;
> +	} else if (of_find_property(pdev->dev.of_node, "mboxes", NULL)) {
>  		zynqmp_pm_init_suspend_work =
>  			devm_kzalloc(&pdev->dev,
>  				     sizeof(struct zynqmp_pm_work_struct),
> @@ -223,6 +263,10 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>  
>  	ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_suspend_mode.attr);
>  	if (ret) {
> +		if (event_registered) {
> +			xlnx_unregister_event(PM_INIT_SUSPEND_CB, 0, 0, suspend_event_callback);
> +			event_registered = false;
> +		}
>  		dev_err(&pdev->dev, "unable to create sysfs interface\n");
>  		return ret;
>  	}
> @@ -233,6 +277,8 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>  static int zynqmp_pm_remove(struct platform_device *pdev)
>  {
>  	sysfs_remove_file(&pdev->dev.kobj, &dev_attr_suspend_mode.attr);
> +	if (event_registered)
> +		xlnx_unregister_event(PM_INIT_SUSPEND_CB, 0, 0, suspend_event_callback);
>  
>  	if (!rx_chan)
>  		mbox_free_channel(rx_chan);
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

RE: [PATCH 2/6] firmware: xilinx: add macros of node ids for error event

[Abhyuday Godhasara]

Hi,

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: Monday, June 21, 2021 4:42 PM
> To: Abhyuday Godhasara <agodhasa@xilinx.com>; Michal Simek
> <michals@xilinx.com>
> Cc: Rajan Vaja <RAJANV@xilinx.com>; Manish Narani <MNARANI@xilinx.com>;
> zou_wei@huawei.com; Amit Sunil Dhamne <amitsuni@xlnx.xilinx.com>; Sai
> Krishna Potthuri <lakshmis@xilinx.com>; Jiaying Liang <jliang@xilinx.com>;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 2/6] firmware: xilinx: add macros of node ids for error
> event
> 
> 
> 
> On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> > Add macros for the Node-Id of Error events.
> >
> > Move supported api callback ids from zynqmp-power to zynqmp-firmware.
> >
> > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> > ---
> >  drivers/soc/xilinx/zynqmp_power.c    |  6 ------
> >  include/linux/firmware/xlnx-zynqmp.h | 13 +++++++++++++
> >  2 files changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/soc/xilinx/zynqmp_power.c
> > b/drivers/soc/xilinx/zynqmp_power.c
> > index c556623..76478fe 100644
> > --- a/drivers/soc/xilinx/zynqmp_power.c
> > +++ b/drivers/soc/xilinx/zynqmp_power.c
> > @@ -46,12 +46,6 @@ static const char *const suspend_modes[] = {
> >
> >  static enum pm_suspend_mode suspend_mode = PM_SUSPEND_MODE_STD;
> >
> > -enum pm_api_cb_id {
> > -	PM_INIT_SUSPEND_CB = 30,
> > -	PM_ACKNOWLEDGE_CB,
> > -	PM_NOTIFY_CB,
> > -};
> > -
> >  static void zynqmp_pm_get_callback_data(u32 *buf)  {
> >  	zynqmp_pm_invoke_fn(GET_CALLBACK_DATA, 0, 0, 0, 0, buf); diff --git
> > a/include/linux/firmware/xlnx-zynqmp.h
> > b/include/linux/firmware/xlnx-zynqmp.h
> > index 6557832..c715756 100644
> > --- a/include/linux/firmware/xlnx-zynqmp.h
> > +++ b/include/linux/firmware/xlnx-zynqmp.h
> > @@ -60,6 +60,19 @@
> >  #define XILINX_ZYNQMP_PM_FPGA_FULL	0x0U
> >  #define XILINX_ZYNQMP_PM_FPGA_PARTIAL	BIT(0)
> >
> > +/*
> > + * Node IDs for the Error Events.
> > + */
> > +#define EVENT_ERROR_PMC_ERR1	(0x28100000U)
> > +#define EVENT_ERROR_PMC_ERR2	(0x28104000U)
> > +#define EVENT_ERROR_PSM_ERR1	(0x28108000U)
> > +#define EVENT_ERROR_PSM_ERR2	(0x2810C000U)
> > +
> > +enum pm_api_cb_id {
> > +	PM_INIT_SUSPEND_CB = 30,
> 
> No information about why PM_ACKNOWLEDGE_CB was removed. Doing it via
> separate patch would be the best.
[Abhyuday] Will not remove PM_ACKNOWLEDGE_CB as to be in sync with firmware.

> 
> M
> 
> > +	PM_NOTIFY_CB = 32,
> > +};
> > +
> >  enum pm_api_id {
> >  	PM_GET_API_VERSION = 1,
> >  	PM_REGISTER_NOTIFIER = 5,
> >

Thanks,
Abhyuday

RE: [PATCH 1/6] firmware: xilinx: add register notifier in zynqmp firmware

[Abhyuday Godhasara]

Hi,

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: Monday, June 21, 2021 5:16 PM
> To: Abhyuday Godhasara <agodhasa@xilinx.com>; Michal Simek
> <michals@xilinx.com>
> Cc: Rajan Vaja <RAJANV@xilinx.com>; Manish Narani <MNARANI@xilinx.com>;
> zou_wei@huawei.com; Amit Sunil Dhamne <amitsuni@xlnx.xilinx.com>; Sai
> Krishna Potthuri <lakshmis@xilinx.com>; Jiaying Liang <jliang@xilinx.com>;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Tejas Patel
> <tejasp@xlnx.xilinx.com>
> Subject: Re: [PATCH 1/6] firmware: xilinx: add register notifier in zynqmp
> firmware
> 
> 
> 
> On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> > In zynqmp-firmware, register notifier is not supported, add support of
> > register notifier in zynqmp-firmware.
> >
> > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> > Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> > ---
> >  drivers/firmware/xilinx/zynqmp-debug.c |  2 +-
> >  drivers/firmware/xilinx/zynqmp.c       | 23 +++++++++++++++++++++++
> >  include/linux/firmware/xlnx-zynqmp.h   | 11 ++++++++++-
> >  3 files changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp-debug.c
> > b/drivers/firmware/xilinx/zynqmp-debug.c
> > index 99606b3..7841cb7 100644
> > --- a/drivers/firmware/xilinx/zynqmp-debug.c
> > +++ b/drivers/firmware/xilinx/zynqmp-debug.c
> > @@ -2,7 +2,7 @@
> >  /*
> >   * Xilinx Zynq MPSoC Firmware layer for debugfs APIs
> >   *
> > - *  Copyright (C) 2014-2018 Xilinx, Inc.
> > + *  Copyright (C) 2014-2021 Xilinx, Inc.
> 
> no reason for this change when you don't change this file.
[Abhyuday] I will remove this in next patch series.

> 
> >   *
> >   *  Michal Simek <michal.simek@xilinx.com>
> >   *  Davorin Mista <davorin.mista@aggios.com> diff --git
> > a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> > index 15b13832..33f190e 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -1037,6 +1037,29 @@ int zynqmp_pm_aes_engine(const u64 address,
> u32
> > *out)  EXPORT_SYMBOL_GPL(zynqmp_pm_aes_engine);
> >
> >  /**
> > + * zynqmp_pm_register_notifier() - PM API for register a subsystem
> > + *                                to be notified about specific
> > + *                                event/error.
> > + * @node:	Node ID to which the event is related.
> > + * @event:	Event Mask of Error events for which wants to get notified.
> > + * @wake:	Wake subsystem upon capturing the event if value 1
> > + * @enable:	Enable the registration for value 1, disable for value 0
> > + *
> > + * This function is used to register/un-register for particular
> > + node-event
> > + * combination in firmware.
> > + *
> > + * Return: Returns status, either success or error+reason */
> > +
> > +int zynqmp_pm_register_notifier(const u32 node, const u32 event,
> > +				const u32 wake, const u32 enable) {
> > +	return zynqmp_pm_invoke_fn(PM_REGISTER_NOTIFIER, node, event,
> > +				   wake, enable, NULL);
> > +}
> > +EXPORT_SYMBOL_GPL(zynqmp_pm_register_notifier);
> > +
> > +/**
> >   * zynqmp_pm_system_shutdown - PM call to request a system shutdown or
> restart
> >   * @type:	Shutdown or restart? 0 for shutdown, 1 for restart
> >   * @subtype:	Specifies which system should be restarted or shut down
> > diff --git a/include/linux/firmware/xlnx-zynqmp.h
> > b/include/linux/firmware/xlnx-zynqmp.h
> > index 9d1a5c1..6557832 100644
> > --- a/include/linux/firmware/xlnx-zynqmp.h
> > +++ b/include/linux/firmware/xlnx-zynqmp.h
> > @@ -2,7 +2,7 @@
> >  /*
> >   * Xilinx Zynq MPSoC Firmware layer
> >   *
> > - *  Copyright (C) 2014-2019 Xilinx
> > + *  Copyright (C) 2014-2021 Xilinx
> >   *
> >   *  Michal Simek <michal.simek@xilinx.com>
> >   *  Davorin Mista <davorin.mista@aggios.com> @@ -62,6 +62,7 @@
> >
> >  enum pm_api_id {
> >  	PM_GET_API_VERSION = 1,
> > +	PM_REGISTER_NOTIFIER = 5,
> >  	PM_SYSTEM_SHUTDOWN = 12,
> >  	PM_REQUEST_NODE = 13,
> >  	PM_RELEASE_NODE = 14,
> > @@ -411,6 +412,8 @@ int zynqmp_pm_pinctrl_get_config(const u32 pin,
> const u32 param,
> >  				 u32 *value);
> >  int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
> >  				 u32 value);
> > +int zynqmp_pm_register_notifier(const u32 node, const u32 event,
> > +				const u32 wake, const u32 enable);
> >  #else
> >  static inline int zynqmp_pm_get_api_version(u32 *version)  { @@
> > -622,6 +625,12 @@ static inline int zynqmp_pm_pinctrl_set_config(const
> > u32 pin, const u32 param,  {
> >  	return -ENODEV;
> >  }
> > +
> > +static inline int zynqmp_pm_register_notifier(const u32 node, const u32
> event,
> > +					      const u32 wake, const u32 enable)
> {
> > +	return -ENODEV;
> > +}
> >  #endif
> >
> >  #endif /* __FIRMWARE_ZYNQMP_H__ */
> >
> 
> M
Thanks,
Abhyuday

RE: [PATCH 4/6] drivers: soc: xilinx: add xilinx event management driver

[Abhyuday Godhasara]

Hi,

Thanks for the review.

> -----Original Message-----
> From: Michal Simek <michal.simek@xilinx.com>
> Sent: Monday, June 21, 2021 5:26 PM
> To: Abhyuday Godhasara <agodhasa@xilinx.com>; Michal Simek
> <michals@xilinx.com>
> Cc: Rajan Vaja <RAJANV@xilinx.com>; Manish Narani <MNARANI@xilinx.com>;
> zou_wei@huawei.com; Amit Sunil Dhamne <amitsuni@xlnx.xilinx.com>; Sai
> Krishna Potthuri <lakshmis@xilinx.com>; Jiaying Liang <jliang@xilinx.com>;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Tejas Patel
> <tejasp@xlnx.xilinx.com>
> Subject: Re: [PATCH 4/6] drivers: soc: xilinx: add xilinx event management
> driver
> 
> 
> 
> On 5/28/21 5:59 PM, Abhyuday Godhasara wrote:
> > Xilinx event management driver provide an interface to subscribe or
> > unsubscribe for the event/callback supported by firmware. Agent can
> > use this driver to register for Error Event, Device Event and Suspend callback.
> > This driver only allow one agent per event to do registration. Driver
> > will return an error in case of multiple registration for the same event.
> >
> > This driver gets notification from firmware through ATF as SGI. During
> > initialization, event manager driver register handler for SGI used for
> > notification. It also provide SGI number info to ATF by using
> > IOCTL_REGISTER_SGI call to ATF.
> >
> > After receiving notification from firmware, driver makes SMC call to
> > ATF to get IPI data. From the IPI data provided by ATF, event manager
> > identified cause of event and forward that event/callback notification
> > to respective subscribed driver. After this, in case of Error Event,
> > driver performs unregistration as firmware expecting from agent to do
> > re-registration if agent wants to get notified on second occurrence of
> > error event.
> >
> > Add new IOCTL id IOCTL_REGISTER_SGI = 25 which use to register SGI on ATF.
> 
> nit: Instead of ATF you should use TF-A
> 
> >
> > Older firmware doesn't have all required support for event handling
> > which is required by event manager driver. So add check for register
> > notifier version in event manager driver.
> >
> > Xilinx event management driver provide support to subscribe for
> > multiple error events with the use of Event Mask in single call of
> > xlnx_register_event(). Agent driver can provide 'Event' parameter
> > value as ORed of multiple event masks to register single callback for
> > multiple events. For example, to register callback for event=0x1 and
> > event=0x2 for the given node, agent can provide event=0x3 (0x1 | 0x2).
> > It is not possible to register multiple event for different nodes in
> > single registration call.
> >
> > Also provide support to receive multiple error events as in single
> > notification from firmware and then forward it to subscribed drivers
> > via registered callback one by one.
> >
> > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com>
> > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > Signed-off-by: Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> > ---
> >  MAINTAINERS                                 |   6 +
> >  drivers/soc/xilinx/Kconfig                  |  10 +
> >  drivers/soc/xilinx/Makefile                 |   1 +
> >  drivers/soc/xilinx/xlnx_event_manager.c     | 598
> ++++++++++++++++++++++++++++
> >  include/linux/firmware/xlnx-event-manager.h |  36 ++
> >  include/linux/firmware/xlnx-zynqmp.h        |   2 +
> >  6 files changed, 653 insertions(+)
> >  create mode 100644 drivers/soc/xilinx/xlnx_event_manager.c
> >  create mode 100644 include/linux/firmware/xlnx-event-manager.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index 3a2384e..6299069 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20096,6 +20096,12 @@ T:	git https://github.com/Xilinx/linux-
> xlnx.git
> >  F:	Documentation/devicetree/bindings/phy/xlnx,zynqmp-psgtr.yaml
> >  F:	drivers/phy/xilinx/phy-zynqmp.c
> >
> > +XILINX EVENT MANAGEMENT DRIVER
> > +M:	Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
> > +S:	Maintained
> > +F:	drivers/soc/xilinx/xlnx_event_manager.c
> > +F:	include/linux/firmware/xlnx-event-manager.h
> > +
> >  XILLYBUS DRIVER
> >  M:	Eli Billauer <eli.billauer@gmail.com>
> >  L:	linux-kernel@vger.kernel.org
> > diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> > index 53af911..8a755a5 100644
> > --- a/drivers/soc/xilinx/Kconfig
> > +++ b/drivers/soc/xilinx/Kconfig
> > @@ -25,4 +25,14 @@ config ZYNQMP_PM_DOMAINS
> >  	  Say yes to enable device power management through PM domains
> >  	  If in doubt, say N.
> >
> > +config XLNX_EVENT_MANAGER
> > +	bool "Enable Xilinx Event Management Driver"
> > +	depends on ZYNQMP_FIRMWARE
> > +	default ZYNQMP_FIRMWARE
> > +	help
> > +	  Say yes to enable event management support for Xilinx.
> > +	  This driver uses firmware driver as an interface for event/power
> > +	  management request to firmware.
> > +
> > +	  If in doubt, say N.
> >  endmenu
> > diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
> > index 9854e6f..41e585b 100644
> > --- a/drivers/soc/xilinx/Makefile
> > +++ b/drivers/soc/xilinx/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_ZYNQMP_POWER)	+= zynqmp_power.o
> >  obj-$(CONFIG_ZYNQMP_PM_DOMAINS) += zynqmp_pm_domains.o
> > +obj-$(CONFIG_XLNX_EVENT_MANAGER)	+= xlnx_event_manager.o
> > diff --git a/drivers/soc/xilinx/xlnx_event_manager.c
> > b/drivers/soc/xilinx/xlnx_event_manager.c
> > new file mode 100644
> > index 0000000..bf726ee
> > --- /dev/null
> > +++ b/drivers/soc/xilinx/xlnx_event_manager.c
> > @@ -0,0 +1,598 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx Event Management Driver
> > + *
> > + *  Copyright (C) 2021 Xilinx, Inc.
> > + *
> > + *  Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>  */
> > +
> > +#include <linux/cpuhotplug.h>
> > +#include <linux/firmware/xlnx-event-manager.h>
> > +#include <linux/firmware/xlnx-zynqmp.h> #include <linux/hashtable.h>
> > +#include <linux/interrupt.h> #include <linux/irq.h> #include
> > +<linux/irqdomain.h> #include <linux/module.h> #include
> > +<linux/of_irq.h> #include <linux/platform_device.h> #include
> > +<linux/slab.h>
> > +
> > +static DEFINE_PER_CPU_READ_MOSTLY(int, cpu_number1);
> > +
> > +static int virq_sgi;
> > +static int event_manager_availability = -EACCES;
> > +
> > +/* SGI number used for Event management driver */
> > +#define XLNX_EVENT_SGI_NUM	(15)
> > +
> > +/* Max number of driver can register for same event */
> > +#define MAX_DRIVER_PER_EVENT	(10U)
> > +
> > +/* Max HashMap Order for PM API feature check (1<<7 = 128) */
> > +#define REGISTERED_DRIVER_MAX_ORDER	(7)
> > +
> > +#define MAX_BITS	(32U) /* Number of bits available for error mask */
> > +
> > +#define FIRMWARE_VERSION_MASK			(0xFFFFU)
> > +#define REGISTER_NOTIFIER_FIRMWARE_VERSION	(2U)
> > +
> > +static DEFINE_HASHTABLE(reg_driver_map,
> REGISTERED_DRIVER_MAX_ORDER);
> > +static int sgi_num = XLNX_EVENT_SGI_NUM;
> 
> make this module parameter with default on XLNX_EVENT_SGI_NUM.
[Abhyuday] I make sgi_num as module parameter with default value XLNX_EVENT_SGI_NUM in next patch series.

> 
> > +
> > +/**
> > + * struct registered_event_data - Registered Event Data.
> > + * @key:		key is the combine id(Node-Id | Event-Id) of type u64
> > + *			where upper u32 for Node-Id and lower u32 for Event-
> Id,
> > + *			And this used as key to index into hashmap.
> > + * @agent_data:		Data passed back to handler function.
> > + * @cb_type:		Type of Api callback, like PM_NOTIFY_CB, etc.
> > + * @eve_cb:		Function pointer to store the callback function.
> > + * @wake:		If this flag set, firmware will wakeup processor if is
> > + *			in sleep or power down state.
> > + * @hentry:		hlist_node that hooks this entry into hashtable.
> > + */
> > +struct registered_event_data {
> > +	u64 key;
> > +	enum pm_api_cb_id cb_type;
> > +	void *agent_data;
> > +
> > +	event_cb_func_t eve_cb;
> > +	bool wake;
> > +	struct hlist_node hentry;
> > +};
> > +
> > +static bool xlnx_is_error_event(const u32 node_id) {
> > +	if (node_id == EVENT_ERROR_PMC_ERR1 ||
> > +	    node_id == EVENT_ERROR_PMC_ERR2 ||
> > +	    node_id == EVENT_ERROR_PSM_ERR1 ||
> > +	    node_id == EVENT_ERROR_PSM_ERR2)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +static int xlnx_add_cb_for_notify_event(const u32 node_id, const u32 event,
> const bool wake,
> > +					event_cb_func_t cb_fun,	void
> *data)
> > +{
> > +	u64 key = 0;
> > +	struct registered_event_data *eve_data;
> > +
> > +	key = ((u64)node_id << 32U) | (u64)event;
> > +	/* Check for existing entry in hash table for given key id */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> > +		if (eve_data->key == key) {
> 
> Don't you need to check eve_data here too?
[Abhyuday] As per https://github.com/torvalds/linux/blob/44db63d1ad8d71c6932cbe007eb41f31c434d140/tools/include/linux/list.h#L714 not required to do NULL check for eve_data, As for loop will break if eve_data got as NULL.

> 
> > +			pr_err("Found as already registered\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	/* Add new entry if not present */
> > +	eve_data = kmalloc(sizeof(*eve_data), GFP_KERNEL);
> > +	if (!eve_data)
> > +		return -ENOMEM;
> > +
> > +	eve_data->key = key;
> > +	eve_data->cb_type = PM_NOTIFY_CB;
> > +	eve_data->eve_cb = cb_fun;
> > +	eve_data->wake = wake;
> > +	eve_data->agent_data = data;
> > +
> > +	hash_add(reg_driver_map, &eve_data->hentry, key);
> > +
> > +	return 0;
> > +}
> > +
> > +static int xlnx_add_cb_for_suspend(event_cb_func_t cb_fun, void
> > +*data) {
> > +	struct registered_event_data *eve_data;
> > +
> > +	/* Check for existing entry in hash table for given cb_type */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry,
> PM_INIT_SUSPEND_CB) {
> > +		if (eve_data->cb_type == PM_INIT_SUSPEND_CB) {
> 
> Same as above to make sure that eve_data is not NULL.
> The same is also below.
[Abhyuday] As per https://github.com/torvalds/linux/blob/44db63d1ad8d71c6932cbe007eb41f31c434d140/tools/include/linux/list.h#L714 not required to do NULL check for eve_data, As for loop will break if eve_data got as NULL.
> 
> > +			pr_err("Found as already registered\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	/* Add new entry if not present */
> > +	eve_data = kmalloc(sizeof(*eve_data), GFP_KERNEL);
> > +	if (!eve_data)
> > +		return -ENOMEM;
> > +
> > +	eve_data->key = 0;
> > +	eve_data->cb_type = PM_INIT_SUSPEND_CB;
> > +	eve_data->eve_cb = cb_fun;
> > +	eve_data->agent_data = data;
> > +
> > +	hash_add(reg_driver_map, &eve_data->hentry,
> PM_INIT_SUSPEND_CB);
> > +
> > +	return 0;
> > +}
> > +
> > +static int xlnx_remove_cb_for_suspend(event_cb_func_t cb_fun) {
> > +	bool is_callback_found = false;
> > +	struct registered_event_data *eve_data;
> > +
> > +	/* Check for existing entry in hash table for given cb_type */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry,
> PM_INIT_SUSPEND_CB) {
> > +		if (eve_data->cb_type == PM_INIT_SUSPEND_CB &&
> > +		    eve_data->eve_cb == cb_fun) {
> > +			is_callback_found = true;
> > +			/* remove an object from a hashtable */
> > +			hash_del(&eve_data->hentry);
> > +			kfree(eve_data);
> > +		}
> > +	}
> > +	if (!is_callback_found) {
> > +		pr_warn("Didn't find any registered callback for suspend
> event\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int xlnx_remove_cb_for_notify_event(const u32 node_id, const u32
> event,
> > +					   event_cb_func_t cb_fun)
> > +{
> > +	bool is_callback_found = false;
> > +	struct registered_event_data *eve_data;
> > +	u64 key = ((u64)node_id << 32U) | (u64)event;
> > +
> > +	/* Check for existing entry in hash table for given key id */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> > +		if (eve_data->key == key &&
> > +		    eve_data->eve_cb == cb_fun) {
> > +			is_callback_found = true;
> > +			/* remove an object from a hashtable */
> > +			hash_del(&eve_data->hentry);
> > +			kfree(eve_data);
> > +		}
> > +	}
> > +	if (!is_callback_found) {
> > +		pr_warn("Didn't find any registered callback for 0x%x 0x%x\n",
> > +			node_id, event);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * xlnx_register_event() - Register for the event.
> > + * @cb_type:	Type of callback from pm_api_cb_id,
> > + *			PM_NOTIFY_CB - for Error Events,
> > + *			PM_INIT_SUSPEND_CB - for suspend callback.
> > + * @node_id:	Node-Id related to event.
> > + * @event:	Event Mask for the Error Event.
> > + * @wake:	Flag specifying whether the subsystem should be woken upon
> > + *		event notification.
> > + * @cb_fun:	Function pointer to store the callback function.
> > + * @data:	Pointer for the driver instance.
> > + *
> > + * Return:	Returns 0 on successful registration else error code.
> > + */
> > +int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32
> node_id, const u32 event,
> > +			const bool wake, event_cb_func_t cb_fun, void *data) {
> > +	int ret = 0;
> > +	u32 eve;
> > +	int pos;
> > +
> > +	if (event_manager_availability)
> > +		return event_manager_availability;
> > +
> > +	if (cb_type != PM_NOTIFY_CB && cb_type != PM_INIT_SUSPEND_CB) {
> > +		pr_err("%s() Unsupported Callback 0x%x\n", __func__,
> cb_type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!cb_fun)
> > +		return -EFAULT;
> > +
> > +	if (cb_type == PM_INIT_SUSPEND_CB) {
> > +		ret = xlnx_add_cb_for_suspend(cb_fun, data);
> > +	} else {
> > +		if (!xlnx_is_error_event(node_id)) {
> > +			/* Add entry for Node-Id/Event in hash table */
> > +			ret = xlnx_add_cb_for_notify_event(node_id, event,
> wake, cb_fun, data);
> > +		} else {
> > +			/* Add into Hash table */
> > +			for (pos = 0; pos < MAX_BITS; pos++) {
> > +				eve = event & (1 << pos);
> > +				if (!eve)
> > +					continue;
> > +
> > +				/* Add entry for Node-Id/Eve in hash table */
> > +				ret = xlnx_add_cb_for_notify_event(node_id,
> eve, wake, cb_fun,
> > +								   data);
> > +				/* Break the loop if got error */
> > +				if (ret)
> > +					break;
> > +			}
> > +			if (ret) {
> > +				/* Skip the Event for which got the error */
> > +				pos--;
> > +				/* Remove registered(during this call) event
> from hash table */
> > +				for ( ; pos >= 0; pos--) {
> > +					eve = event & (1 << pos);
> > +					if (!eve)
> > +						continue;
> > +
> 	xlnx_remove_cb_for_notify_event(node_id, eve, cb_fun);
> > +				}
> > +			}
> > +		}
> > +
> > +		if (ret) {
> > +			pr_err("%s() failed for 0x%x and 0x%x: %d\r\n",
> __func__, node_id,
> > +			       event, ret);
> > +			return ret;
> > +		}
> > +
> > +		/* Register for Node-Id/Event combination in firmware */
> > +		ret = zynqmp_pm_register_notifier(node_id, event, wake,
> true);
> > +		if (ret) {
> > +			pr_err("%s() failed for 0x%x and 0x%x: %d\r\n",
> __func__, node_id,
> > +			       event, ret);
> > +			/* Remove already registered event from hash table */
> > +			if (xlnx_is_error_event(node_id)) {
> > +				for (pos = 0; pos < MAX_BITS; pos++) {
> > +					eve = event & (1 << pos);
> > +					if (!eve)
> > +						continue;
> > +
> 	xlnx_remove_cb_for_notify_event(node_id, eve, cb_fun);
> > +				}
> > +			} else {
> > +				xlnx_remove_cb_for_notify_event(node_id,
> event, cb_fun);
> > +			}
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(xlnx_register_event);
> > +
> > +/**
> > + * xlnx_unregister_event() - Unregister for the event.
> > + * @cb_type:	Type of callback from pm_api_cb_id,
> > + *			PM_NOTIFY_CB - for Error Events,
> > + *			PM_INIT_SUSPEND_CB - for suspend callback.
> > + * @node_id:	Node-Id related to event.
> > + * @event:	Event Mask for the Error Event.
> > + * @cb_fun:	Function pointer of callback function.
> > + *
> > + * Return:	Returns 0 on successful unregistration else error code.
> > + */
> > +int xlnx_unregister_event(const enum pm_api_cb_id cb_type, const u32
> node_id, const u32 event,
> > +			  event_cb_func_t cb_fun)
> > +{
> > +	int ret;
> > +	u32 eve, pos;
> > +
> > +	if (event_manager_availability)
> > +		return event_manager_availability;
> > +
> > +	if (cb_type != PM_NOTIFY_CB && cb_type != PM_INIT_SUSPEND_CB) {
> > +		pr_err("%s() Unsupported Callback 0x%x\n", __func__,
> cb_type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!cb_fun)
> > +		return -EFAULT;
> > +
> > +	if (cb_type == PM_INIT_SUSPEND_CB) {
> > +		ret = xlnx_remove_cb_for_suspend(cb_fun);
> > +	} else {
> > +		/* Remove Node-Id/Event from hash table */
> > +		if (!xlnx_is_error_event(node_id)) {
> > +			xlnx_remove_cb_for_notify_event(node_id, event,
> cb_fun);
> > +		} else {
> > +			for (pos = 0; pos < MAX_BITS; pos++) {
> > +				eve = event & (1 << pos);
> > +				if (!eve)
> > +					continue;
> > +
> > +				xlnx_remove_cb_for_notify_event(node_id,
> eve, cb_fun);
> > +			}
> > +		}
> > +
> > +		/* Un-register for Node-Id/Event combination */
> > +		ret = zynqmp_pm_register_notifier(node_id, event, false,
> false);
> > +		if (ret) {
> > +			pr_err("%s() failed for 0x%x and 0x%x: %d\n",
> > +			       __func__, node_id, event, ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(xlnx_unregister_event);
> > +
> > +static void xlnx_call_suspend_cb_handler(const u32 *payload) {
> > +	bool is_callback_found = false;
> > +	struct registered_event_data *eve_data;
> > +	u32 cb_type = payload[0];
> > +
> > +	/* Check for existing entry in hash table for given cb_type */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry, cb_type) {
> > +		if (eve_data->cb_type == cb_type) {
> > +			eve_data->eve_cb(&payload[0], eve_data-
> >agent_data);
> > +			is_callback_found = true;
> > +		}
> > +	}
> > +	if (!is_callback_found)
> > +		pr_warn("Didn't find any registered callback for suspend
> event\n");
> > +}
> > +
> > +static void xlnx_call_notify_cb_handler(const u32 *payload) {
> > +	bool is_callback_found = false;
> > +	struct registered_event_data *eve_data;
> > +	u64 key = ((u64)payload[1] << 32U) | (u64)payload[2];
> > +	int ret;
> > +
> > +	/* Check for existing entry in hash table for given key id */
> > +	hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> > +		if (eve_data->key == key) {
> > +			eve_data->eve_cb(&payload[0], eve_data-
> >agent_data);
> > +			is_callback_found = true;
> > +
> > +			/* re register with firmware to get future events */
> > +			ret = zynqmp_pm_register_notifier(payload[1],
> payload[2],
> > +							  eve_data->wake,
> true);
> > +			if (ret) {
> > +				pr_err("%s() failed for 0x%x and 0x%x:
> %d\r\n", __func__,
> > +				       payload[1], payload[2], ret);
> > +				/* Remove already registered event from hash
> table */
> > +				xlnx_remove_cb_for_notify_event(payload[1],
> payload[2],
> > +								eve_data-
> >eve_cb);
> > +			}
> > +		}
> > +	}
> > +	if (!is_callback_found)
> > +		pr_warn("Didn't find any registered callback for 0x%x 0x%x\n",
> > +			payload[1], payload[2]);
> > +}
> > +
> > +static void xlnx_get_event_callback_data(u32 *buf) {
> > +	zynqmp_pm_invoke_fn(GET_CALLBACK_DATA, 0, 0, 0, 0, buf); }
> > +
> > +static irqreturn_t xlnx_event_handler(int irq, void *dev_id) {
> > +	u32 cb_type, node_id, event, pos;
> > +	u32 payload[CB_MAX_PAYLOAD_SIZE] = {0};
> > +	u32 event_data[CB_MAX_PAYLOAD_SIZE] = {0};
> > +
> > +	/* Get event data */
> > +	xlnx_get_event_callback_data(payload);
> > +
> > +	/* First element is callback type, others are callback arguments */
> > +	cb_type = payload[0];
> > +
> > +	if (cb_type == PM_NOTIFY_CB) {
> > +		node_id = payload[1];
> > +		event = payload[2];
> > +		if (!xlnx_is_error_event(node_id)) {
> > +			xlnx_call_notify_cb_handler(payload);
> > +		} else {
> > +			/*
> > +			 * Each call back function expecting payload as an
> input arguments.
> > +			 * We can get multiple error events as in one call back
> through error
> > +			 * mask. So payload[2] may can contain multiple error
> events.
> > +			 * In reg_driver_map database we store data in the
> combination of single
> > +			 * node_id-error combination.
> > +			 * So coping the payload message into event_data and
> update the
> > +			 * event_data[2] with Error Mask for single error event
> and use
> > +			 * event_data as input argument for registered call
> back function.
> > +			 *
> > +			 */
> > +			memcpy(event_data, payload, (4 *
> CB_MAX_PAYLOAD_SIZE));
> > +			/* Support Multiple Error Event */
> > +			for (pos = 0; pos < MAX_BITS; pos++) {
> > +				if ((0 == (event & (1 << pos))))
> > +					continue;
> > +				event_data[2] = (event & (1 << pos));
> > +				xlnx_call_notify_cb_handler(event_data);
> > +			}
> > +		}
> > +	} else if (cb_type == PM_INIT_SUSPEND_CB) {
> > +		xlnx_call_suspend_cb_handler(payload);
> > +	} else {
> > +		pr_err("%s() Unsupported Callback %d\n", __func__, cb_type);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int xlnx_event_cpuhp_start(unsigned int cpu) {
> > +	enable_percpu_irq(virq_sgi, IRQ_TYPE_NONE);
> > +
> > +	return 0;
> > +}
> > +
> > +static int xlnx_event_cpuhp_down(unsigned int cpu) {
> > +	disable_percpu_irq(virq_sgi);
> > +
> > +	return 0;
> > +}
> > +
> > +static void xlnx_disable_percpu_irq(void *data) {
> > +	disable_percpu_irq(virq_sgi);
> > +}
> > +
> > +static int xlnx_event_init_sgi(struct platform_device *pdev) {
> > +	int ret = 0;
> > +	int cpu = smp_processor_id();
> > +	/*
> > +	 * IRQ related structures are used for the following:
> > +	 * for each SGI interrupt ensure its mapped by GIC IRQ domain
> > +	 * and that each corresponding linux IRQ for the HW IRQ has
> > +	 * a handler for when receiving an interrupt from the remote
> > +	 * processor.
> > +	 */
> > +	struct irq_domain *domain;
> > +	struct irq_fwspec sgi_fwspec;
> > +	struct device_node *interrupt_parent = NULL;
> > +	struct device *parent = pdev->dev.parent;
> > +
> > +	/* Find GIC controller to map SGIs. */
> > +	interrupt_parent = of_irq_find_parent(parent->of_node);
> > +	if (!interrupt_parent) {
> > +		dev_err(&pdev->dev, "Failed to find property for Interrupt
> parent\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Each SGI needs to be associated with GIC's IRQ domain. */
> > +	domain = irq_find_host(interrupt_parent);
> > +	of_node_put(interrupt_parent);
> > +
> > +	/* Each mapping needs GIC domain when finding IRQ mapping. */
> > +	sgi_fwspec.fwnode = domain->fwnode;
> > +
> > +	/*
> > +	 * When irq domain looks at mapping each arg is as follows:
> > +	 * 3 args for: interrupt type (SGI), interrupt # (set later), type
> > +	 */
> > +	sgi_fwspec.param_count = 1;
> > +
> > +	/* Set SGI's hwirq */
> > +	sgi_fwspec.param[0] = sgi_num;
> > +	virq_sgi = irq_create_fwspec_mapping(&sgi_fwspec);
> > +
> > +	per_cpu(cpu_number1, cpu) = cpu;
> > +	ret = request_percpu_irq(virq_sgi, xlnx_event_handler,
> "xlnx_event_mgmt",
> > +				 &cpu_number1);
> > +	WARN_ON(ret);
> > +	if (ret) {
> > +		irq_dispose_mapping(virq_sgi);
> > +		return ret;
> > +	}
> > +
> > +	irq_to_desc(virq_sgi);
> > +	irq_set_status_flags(virq_sgi, IRQ_PER_CPU);
> > +
> > +	return ret;
> > +}
> > +
> > +static void xlnx_event_cleanup_sgi(struct platform_device *pdev) {
> > +	int cpu = smp_processor_id();
> > +
> > +	per_cpu(cpu_number1, cpu) = cpu;
> > +
> > +	cpuhp_remove_state(CPUHP_AP_ONLINE_DYN);
> > +
> > +	on_each_cpu(xlnx_disable_percpu_irq, NULL, 1);
> > +
> > +	irq_clear_status_flags(virq_sgi, IRQ_PER_CPU);
> > +	free_percpu_irq(virq_sgi, &cpu_number1);
> > +	irq_dispose_mapping(virq_sgi);
> > +}
> > +
> > +static int xlnx_event_manager_probe(struct platform_device *pdev) {
> > +	int ret;
> > +
> > +	ret = zynqmp_pm_feature(PM_REGISTER_NOTIFIER);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "Feature check failed with %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	if ((ret & FIRMWARE_VERSION_MASK) <
> > +	    REGISTER_NOTIFIER_FIRMWARE_VERSION) {
> > +		dev_err(&pdev->dev, "Register notifier version error. Expected
> Firmware: v%d - Found: v%d\n",
> > +			REGISTER_NOTIFIER_FIRMWARE_VERSION,
> > +			ret & FIRMWARE_VERSION_MASK);
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	/* Initialize the SGI */
> > +	ret = xlnx_event_init_sgi(pdev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "SGI Init has been failed with %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Setup function for the CPU hot-plug cases */
> > +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/event:starting",
> > +			  xlnx_event_cpuhp_start, xlnx_event_cpuhp_down);
> > +
> > +	ret = zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_REGISTER_SGI,
> sgi_num,
> > +				  0, NULL);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "SGI Registration over ATF failed with
> %d\n", ret);
> > +		xlnx_event_cleanup_sgi(pdev);
> > +		return ret;
> > +	}
> > +
> > +	event_manager_availability = 0;
> > +
> > +	dev_info(&pdev->dev, "Xilinx Event Management driver probed\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int xlnx_event_manager_remove(struct platform_device *pdev) {
> > +	int i;
> > +	struct registered_event_data *eve_data;
> > +	struct hlist_node *tmp;
> > +	int ret;
> > +
> > +	hash_for_each_safe(reg_driver_map, i, tmp, eve_data, hentry) {
> > +		hash_del(&eve_data->hentry);
> > +		kfree(eve_data);
> > +	}
> > +
> > +	ret = zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_REGISTER_SGI, 0, 1,
> NULL);
> > +	if (ret)
> > +		dev_err(&pdev->dev, "SGI unregistration over ATF failed with
> %d\n",
> > +ret);
> > +
> > +	xlnx_event_cleanup_sgi(pdev);
> > +
> > +	event_manager_availability = -EACCES;
> > +
> > +	return ret;
> > +}
> > +
> > +static struct platform_driver xlnx_event_manager_driver = {
> > +	.probe = xlnx_event_manager_probe,
> > +	.remove = xlnx_event_manager_remove,
> > +	.driver = {
> > +		.name = "xlnx_event_manager",
> > +	},
> > +};
> > +module_platform_driver(xlnx_event_manager_driver);
> > diff --git a/include/linux/firmware/xlnx-event-manager.h
> > b/include/linux/firmware/xlnx-event-manager.h
> > new file mode 100644
> > index 0000000..09758ab
> > --- /dev/null
> > +++ b/include/linux/firmware/xlnx-event-manager.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _FIRMWARE_XLNX_EVENT_MANAGER_H_ #define
> > +_FIRMWARE_XLNX_EVENT_MANAGER_H_
> > +
> > +#include <linux/firmware/xlnx-zynqmp.h>
> > +
> > +#define CB_MAX_PAYLOAD_SIZE	(4U) /*In payload maximum 32bytes
> */
> > +
> > +/************************** Exported Function
> > +*****************************/
> > +
> > +typedef void (*event_cb_func_t)(const u32 *payload, void *data);
> > +
> > +#if IS_REACHABLE(CONFIG_XLNX_EVENT_MANAGER)
> > +int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32
> node_id,
> > +			const u32 event, const bool wake,
> > +			event_cb_func_t cb_fun, void *data);
> > +
> > +int xlnx_unregister_event(const enum pm_api_cb_id cb_type, const u32
> node_id,
> > +			  const u32 event, event_cb_func_t cb_fun); #else
> static inline
> > +int xlnx_register_event(const enum pm_api_cb_id cb_type, const u32
> node_id,
> > +				      const u32 event, const bool wake,
> > +				      event_cb_func_t cb_fun, void *data) {
> > +	return -ENODEV;
> > +}
> > +
> > +istatic inline int xlnx_unregister_event(const enum pm_api_cb_id cb_type,
> const u32 node_id,
> > +					 const u32 event, event_cb_func_t
> cb_fun) {
> > +	return -ENODEV;
> > +}
> > +#endif
> > +
> > +#endif /* _FIRMWARE_XLNX_EVENT_MANAGER_H_ */
> > diff --git a/include/linux/firmware/xlnx-zynqmp.h
> > b/include/linux/firmware/xlnx-zynqmp.h
> > index 06ea5a4..f3e857c 100644
> > --- a/include/linux/firmware/xlnx-zynqmp.h
> > +++ b/include/linux/firmware/xlnx-zynqmp.h
> > @@ -133,6 +133,8 @@ enum pm_ioctl_id {
> >  	IOCTL_READ_PGGS = 15,
> >  	/* Set healthy bit value */
> >  	IOCTL_SET_BOOT_HEALTH_STATUS = 17,
> > +	/* Register SGI to ATF */
> > +	IOCTL_REGISTER_SGI = 25,
> >  };
> >
> >  enum pm_query_id {
> >
> 
> The rest looks good to me. Please push interface to TFA first to make sure that
> API won't change.
[Abhyuday] TFA changes are already merged. 
> 
> Thanks,
> Michal
Thanks,
Abhyuday