[PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface

[PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface

[Jolly Shah]

This patch series adds xilinx specific sysfs interface for below
purposes:
- Register access
- Set shutdown scope
- Set boot health status bit

Rajan Vaja (5):
  firmware: xilinx: Adds new eemi call for reg access
  firmware: xilinx: Add sysfs interface
  firmware: xilinx: Add system shutdown API interface
  firmware: xilinx: Add sysfs to set shutdown scope
  firmware: xilinx: Add sysfs and IOCTL to set boot health status

 Documentation/ABI/stable/sysfs-firmware-zynqmp | 103 +++++++
 drivers/firmware/xilinx/Makefile               |   2 +-
 drivers/firmware/xilinx/zynqmp-ggs.c           | 289 ++++++++++++++++++
 drivers/firmware/xilinx/zynqmp.c               | 392 +++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h           |  37 ++-
 5 files changed, 821 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-firmware-zynqmp
 create mode 100644 drivers/firmware/xilinx/zynqmp-ggs.c

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] firmware: xilinx: Adds new eemi call for reg access

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

This patch adds new EEMI call which is used for CSU/PMU register
access from linux.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 drivers/firmware/xilinx/zynqmp.c     | 183 +++++++++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h |   9 ++
 2 files changed, 192 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index fd3d837..56431ad 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -24,6 +24,8 @@
 #include <linux/firmware/xlnx-zynqmp.h>
 #include "zynqmp-debug.h"
 
+static unsigned long register_address;
+
 static const struct zynqmp_eemi_ops *eemi_ops_tbl;
 
 static const struct mfd_cell firmware_devs[] = {
@@ -664,6 +666,26 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
 				   qos, ack, NULL);
 }
 
+/**
+ * zynqmp_pm_config_reg_access - PM Config API for Config register access
+ * @register_access_id:	ID of the requested REGISTER_ACCESS
+ * @address:		Address of the register to be accessed
+ * @mask:		Mask to be written to the register
+ * @value:		Value to be written to the register
+ * @out:		Returned output value
+ *
+ * This function calls REGISTER_ACCESS to configure CSU/PMU registers.
+ *
+ * Return:	Returns status, either success or error+reason
+ */
+
+static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
+				       u32 mask, u32 value, u32 *out)
+{
+	return zynqmp_pm_invoke_fn(PM_REGISTER_ACCESS, register_access_id,
+				   address, mask, value, out);
+}
+
 static const struct zynqmp_eemi_ops eemi_ops = {
 	.get_api_version = zynqmp_pm_get_api_version,
 	.get_chipid = zynqmp_pm_get_chipid,
@@ -687,6 +709,7 @@ static const struct zynqmp_eemi_ops eemi_ops = {
 	.set_requirement = zynqmp_pm_set_requirement,
 	.fpga_load = zynqmp_pm_fpga_load,
 	.fpga_get_status = zynqmp_pm_fpga_get_status,
+	.register_access = zynqmp_pm_config_reg_access,
 };
 
 /**
@@ -704,6 +727,160 @@ const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
 
+/**
+ * config_reg_store - Write config_reg sysfs attribute
+ * @kobj:	Kobject structure
+ * @attr:	Kobject attribute structure
+ * @buf:	User entered health_status attribute string
+ * @count:	Buffer size
+ *
+ * User-space interface for setting the config register.
+ *
+ * To write any CSU/PMU register
+ * echo <address> <mask> <values> > /sys/firmware/zynqmp/config_reg
+ * Usage:
+ * echo 0x345AB234 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/config_reg
+ *
+ * To Read any CSU/PMU register, write address to the variable like below
+ * echo <address> > /sys/firmware/zynqmp/config_reg
+ *
+ * Return:	count argument if request succeeds, the corresponding error
+ *		code otherwise
+ */
+static ssize_t config_reg_store(struct kobject *kobj,
+				struct kobj_attribute *attr,
+				const char *buf, size_t count)
+{
+	char *kern_buff, *inbuf, *tok;
+	unsigned long address, value, mask;
+	int ret;
+
+	kern_buff = kzalloc(count, GFP_KERNEL);
+	if (!kern_buff)
+		return -ENOMEM;
+
+	ret = strlcpy(kern_buff, buf, count);
+	if (ret < 0) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	inbuf = kern_buff;
+
+	/* Read the addess */
+	tok = strsep(&inbuf, " ");
+	if (!tok) {
+		ret = -EFAULT;
+		goto err;
+	}
+	ret = kstrtol(tok, 16, &address);
+	if (ret) {
+		ret = -EFAULT;
+		goto err;
+	}
+	/* Read the write value */
+	tok = strsep(&inbuf, " ");
+	/*
+	 * If parameter provided is only address, then its a read operation.
+	 * Store the address in a global variable and retrieve whenever
+	 * required.
+	 */
+	if (!tok) {
+		register_address = address;
+		goto err;
+	}
+	register_address = address;
+
+	ret = kstrtol(tok, 16, &mask);
+	if (ret) {
+		ret = -EFAULT;
+		goto err;
+	}
+	tok = strsep(&inbuf, " ");
+	if (!tok) {
+		ret = -EFAULT;
+		goto err;
+	}
+	ret = kstrtol(tok, 16, &value);
+	if (!tok) {
+		ret = -EFAULT;
+		goto err;
+	}
+	ret = zynqmp_pm_config_reg_access(CONFIG_REG_WRITE, address,
+					  mask, value, NULL);
+	if (ret)
+		pr_err("unable to write value to %lx\n", value);
+err:
+	kfree(kern_buff);
+	if (ret)
+		return ret;
+	return count;
+}
+
+/**
+ * config_reg_show - Read config_reg sysfs attribute
+ * @kobj:	Kobject structure
+ * @attr:	Kobject attribute structure
+ * @buf:	User entered health_status attribute string
+ *
+ * User-space interface for getting the config register.
+ *
+ * To Read any CSU/PMU register, write address to the variable like below
+ * echo <address> > /sys/firmware/zynqmp/config_reg
+ *
+ * Then Read the address using below command
+ * cat /sys/firmware/zynqmp/config_reg
+ *
+ * Return: number of chars written to buf.
+ */
+static ssize_t config_reg_show(struct kobject *kobj,
+			       struct kobj_attribute *attr,
+			       char *buf)
+{
+	int ret;
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+
+	ret = zynqmp_pm_config_reg_access(CONFIG_REG_READ, register_address,
+					  0, 0, ret_payload);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "0x%x\n", ret_payload[1]);
+}
+
+static struct kobj_attribute zynqmp_attr_config_reg =
+					__ATTR_RW(config_reg);
+
+static struct attribute *attrs[] = {
+	&zynqmp_attr_config_reg.attr,
+	NULL,
+};
+
+static const struct attribute_group attr_group = {
+	.attrs = attrs,
+	NULL,
+};
+
+static int zynqmp_pm_sysfs_init(void)
+{
+	struct kobject *zynqmp_kobj;
+	int ret;
+
+	zynqmp_kobj = kobject_create_and_add("zynqmp", firmware_kobj);
+	if (!zynqmp_kobj) {
+		pr_err("zynqmp: Firmware kobj add failed.\n");
+		return -ENOMEM;
+	}
+
+	ret = sysfs_create_group(zynqmp_kobj, &attr_group);
+	if (ret) {
+		pr_err("%s() sysfs creation fail with error %d\n",
+		       __func__, ret);
+	}
+
+	return ret;
+}
+
 static int zynqmp_firmware_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -748,6 +925,12 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
 	/* Assign eemi_ops_table */
 	eemi_ops_tbl = &eemi_ops;
 
+	ret = zynqmp_pm_sysfs_init();
+	if (ret) {
+		pr_err("%s() sysfs init fail with error %d\n", __func__, ret);
+		return ret;
+	}
+
 	zynqmp_pm_api_debugfs_init();
 
 	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, firmware_devs,
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index df366f1..55561d0 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -77,6 +77,8 @@ enum pm_api_id {
 	PM_CLOCK_GETRATE,
 	PM_CLOCK_SETPARENT,
 	PM_CLOCK_GETPARENT,
+	/* PM_REGISTER_ACCESS API */
+	PM_REGISTER_ACCESS = 52,
 };
 
 /* PMU-FW return status codes */
@@ -261,6 +263,11 @@ enum tap_delay_type {
 	PM_TAPDELAY_OUTPUT,
 };
 
+enum pm_register_access_id {
+	CONFIG_REG_WRITE,
+	CONFIG_REG_READ,
+};
+
 /**
  * struct zynqmp_pm_query_data - PM query data
  * @qid:	query ID
@@ -305,6 +312,8 @@ struct zynqmp_eemi_ops {
 			       const u32 capabilities,
 			       const u32 qos,
 			       const enum zynqmp_pm_request_ack ack);
+	int (*register_access)(u32 register_access_id, u32 address,
+			       u32 mask, u32 value, u32 *out);
 };
 
 int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] firmware: xilinx: Adds new eemi call for reg access

[Michal Simek]

On 05. 12. 19 0:29, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> This patch adds new EEMI call which is used for CSU/PMU register
> access from linux.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp.c     | 183 +++++++++++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h |   9 ++
>  2 files changed, 192 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index fd3d837..56431ad 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -24,6 +24,8 @@
>  #include <linux/firmware/xlnx-zynqmp.h>
>  #include "zynqmp-debug.h"
>  
> +static unsigned long register_address;
> +
>  static const struct zynqmp_eemi_ops *eemi_ops_tbl;
>  
>  static const struct mfd_cell firmware_devs[] = {
> @@ -664,6 +666,26 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
>  				   qos, ack, NULL);
>  }
>  
> +/**
> + * zynqmp_pm_config_reg_access - PM Config API for Config register access
> + * @register_access_id:	ID of the requested REGISTER_ACCESS
> + * @address:		Address of the register to be accessed
> + * @mask:		Mask to be written to the register
> + * @value:		Value to be written to the register
> + * @out:		Returned output value
> + *
> + * This function calls REGISTER_ACCESS to configure CSU/PMU registers.
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +
> +static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
> +				       u32 mask, u32 value, u32 *out)
> +{
> +	return zynqmp_pm_invoke_fn(PM_REGISTER_ACCESS, register_access_id,
> +				   address, mask, value, out);
> +}
> +
>  static const struct zynqmp_eemi_ops eemi_ops = {
>  	.get_api_version = zynqmp_pm_get_api_version,
>  	.get_chipid = zynqmp_pm_get_chipid,
> @@ -687,6 +709,7 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>  	.set_requirement = zynqmp_pm_set_requirement,
>  	.fpga_load = zynqmp_pm_fpga_load,
>  	.fpga_get_status = zynqmp_pm_fpga_get_status,
> +	.register_access = zynqmp_pm_config_reg_access,
>  };
>  
>  /**
> @@ -704,6 +727,160 @@ const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
>  }
>  EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
>  
> +/**
> + * config_reg_store - Write config_reg sysfs attribute
> + * @kobj:	Kobject structure
> + * @attr:	Kobject attribute structure
> + * @buf:	User entered health_status attribute string
> + * @count:	Buffer size
> + *
> + * User-space interface for setting the config register.
> + *
> + * To write any CSU/PMU register
> + * echo <address> <mask> <values> > /sys/firmware/zynqmp/config_reg
> + * Usage:
> + * echo 0x345AB234 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/config_reg
> + *
> + * To Read any CSU/PMU register, write address to the variable like below
> + * echo <address> > /sys/firmware/zynqmp/config_reg
> + *
> + * Return:	count argument if request succeeds, the corresponding error
> + *		code otherwise
> + */
> +static ssize_t config_reg_store(struct kobject *kobj,
> +				struct kobj_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	char *kern_buff, *inbuf, *tok;
> +	unsigned long address, value, mask;
> +	int ret;
> +
> +	kern_buff = kzalloc(count, GFP_KERNEL);
> +	if (!kern_buff)
> +		return -ENOMEM;
> +
> +	ret = strlcpy(kern_buff, buf, count);
> +	if (ret < 0) {
> +		ret = -EFAULT;
> +		goto err;
> +	}

Greg: What's the recommended way how to parse parameters via sysfs?
I see that kstrndup is used for cloning buffer instead of kzalloc
followed by strchr and sscanf.


> +
> +	inbuf = kern_buff;
> +
> +	/* Read the addess */

typo here.

> +	tok = strsep(&inbuf, " ");
> +	if (!tok) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	ret = kstrtol(tok, 16, &address);
> +	if (ret) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	/* Read the write value */
> +	tok = strsep(&inbuf, " ");
> +	/*
> +	 * If parameter provided is only address, then its a read operation.
> +	 * Store the address in a global variable and retrieve whenever
> +	 * required.
> +	 */
> +	if (!tok) {
> +		register_address = address;
> +		goto err;
> +	}
> +	register_address = address;
> +
> +	ret = kstrtol(tok, 16, &mask);
> +	if (ret) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	tok = strsep(&inbuf, " ");
> +	if (!tok) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	ret = kstrtol(tok, 16, &value);
> +	if (!tok) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	ret = zynqmp_pm_config_reg_access(CONFIG_REG_WRITE, address,
> +					  mask, value, NULL);
> +	if (ret)
> +		pr_err("unable to write value to %lx\n", value);
> +err:
> +	kfree(kern_buff);
> +	if (ret)
> +		return ret;
> +	return count;
> +}
> +
> +/**
> + * config_reg_show - Read config_reg sysfs attribute
> + * @kobj:	Kobject structure
> + * @attr:	Kobject attribute structure
> + * @buf:	User entered health_status attribute string
> + *
> + * User-space interface for getting the config register.
> + *
> + * To Read any CSU/PMU register, write address to the variable like below
> + * echo <address> > /sys/firmware/zynqmp/config_reg
> + *
> + * Then Read the address using below command
> + * cat /sys/firmware/zynqmp/config_reg
> + *
> + * Return: number of chars written to buf.
> + */
> +static ssize_t config_reg_show(struct kobject *kobj,
> +			       struct kobj_attribute *attr,
> +			       char *buf)
> +{
> +	int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +
> +	ret = zynqmp_pm_config_reg_access(CONFIG_REG_READ, register_address,
> +					  0, 0, ret_payload);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "0x%x\n", ret_payload[1]);
> +}
> +
> +static struct kobj_attribute zynqmp_attr_config_reg =
> +					__ATTR_RW(config_reg);


use DEVICE_ATTR_RW() instead

> +
> +static struct attribute *attrs[] = {
> +	&zynqmp_attr_config_reg.attr,
> +	NULL,
> +};

> +
> +static const struct attribute_group attr_group = {
> +	.attrs = attrs,
> +	NULL,
> +};

ATTRIBUTE_GROUPS instead.

> +
> +static int zynqmp_pm_sysfs_init(void)
> +{
> +	struct kobject *zynqmp_kobj;
> +	int ret;
> +
> +	zynqmp_kobj = kobject_create_and_add("zynqmp", firmware_kobj);
> +	if (!zynqmp_kobj) {
> +		pr_err("zynqmp: Firmware kobj add failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	ret = sysfs_create_group(zynqmp_kobj, &attr_group);
> +	if (ret) {
> +		pr_err("%s() sysfs creation fail with error %d\n",
> +		       __func__, ret);

if you fail here you should free kobject_put(zynqmp_kobj);


> +	}
> +
> +	return ret;
> +}
> +
>  static int zynqmp_firmware_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -748,6 +925,12 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
>  	/* Assign eemi_ops_table */
>  	eemi_ops_tbl = &eemi_ops;
>  
> +	ret = zynqmp_pm_sysfs_init();
> +	if (ret) {
> +		pr_err("%s() sysfs init fail with error %d\n", __func__, ret);
> +		return ret;
> +	}
> +
>  	zynqmp_pm_api_debugfs_init();
>  
>  	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, firmware_devs,
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index df366f1..55561d0 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -77,6 +77,8 @@ enum pm_api_id {
>  	PM_CLOCK_GETRATE,
>  	PM_CLOCK_SETPARENT,
>  	PM_CLOCK_GETPARENT,
> +	/* PM_REGISTER_ACCESS API */
> +	PM_REGISTER_ACCESS = 52,
>  };
>  
>  /* PMU-FW return status codes */
> @@ -261,6 +263,11 @@ enum tap_delay_type {
>  	PM_TAPDELAY_OUTPUT,
>  };
>  
> +enum pm_register_access_id {
> +	CONFIG_REG_WRITE,
> +	CONFIG_REG_READ,
> +};
> +
>  /**
>   * struct zynqmp_pm_query_data - PM query data
>   * @qid:	query ID
> @@ -305,6 +312,8 @@ struct zynqmp_eemi_ops {
>  			       const u32 capabilities,
>  			       const u32 qos,
>  			       const enum zynqmp_pm_request_ack ack);
> +	int (*register_access)(u32 register_access_id, u32 address,
> +			       u32 mask, u32 value, u32 *out);
>  };
>  
>  int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] firmware: xilinx: Adds new eemi call for reg access

[Michal Simek]

On 18. 12. 19 15:10, Michal Simek wrote:
> On 05. 12. 19 0:29, Jolly Shah wrote:
>> From: Rajan Vaja <rajan.vaja@xilinx.com>
>>
>> This patch adds new EEMI call which is used for CSU/PMU register
>> access from linux.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
>> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
>> ---
>>  drivers/firmware/xilinx/zynqmp.c     | 183 +++++++++++++++++++++++++++++++++++
>>  include/linux/firmware/xlnx-zynqmp.h |   9 ++
>>  2 files changed, 192 insertions(+)
>>
>> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
>> index fd3d837..56431ad 100644
>> --- a/drivers/firmware/xilinx/zynqmp.c
>> +++ b/drivers/firmware/xilinx/zynqmp.c
>> @@ -24,6 +24,8 @@
>>  #include <linux/firmware/xlnx-zynqmp.h>
>>  #include "zynqmp-debug.h"
>>  
>> +static unsigned long register_address;
>> +
>>  static const struct zynqmp_eemi_ops *eemi_ops_tbl;
>>  
>>  static const struct mfd_cell firmware_devs[] = {
>> @@ -664,6 +666,26 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
>>  				   qos, ack, NULL);
>>  }
>>  
>> +/**
>> + * zynqmp_pm_config_reg_access - PM Config API for Config register access
>> + * @register_access_id:	ID of the requested REGISTER_ACCESS
>> + * @address:		Address of the register to be accessed
>> + * @mask:		Mask to be written to the register
>> + * @value:		Value to be written to the register
>> + * @out:		Returned output value
>> + *
>> + * This function calls REGISTER_ACCESS to configure CSU/PMU registers.
>> + *
>> + * Return:	Returns status, either success or error+reason
>> + */
>> +
>> +static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
>> +				       u32 mask, u32 value, u32 *out)
>> +{
>> +	return zynqmp_pm_invoke_fn(PM_REGISTER_ACCESS, register_access_id,
>> +				   address, mask, value, out);
>> +}
>> +
>>  static const struct zynqmp_eemi_ops eemi_ops = {
>>  	.get_api_version = zynqmp_pm_get_api_version,
>>  	.get_chipid = zynqmp_pm_get_chipid,
>> @@ -687,6 +709,7 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>>  	.set_requirement = zynqmp_pm_set_requirement,
>>  	.fpga_load = zynqmp_pm_fpga_load,
>>  	.fpga_get_status = zynqmp_pm_fpga_get_status,
>> +	.register_access = zynqmp_pm_config_reg_access,
>>  };
>>  
>>  /**
>> @@ -704,6 +727,160 @@ const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
>>  }
>>  EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
>>  
>> +/**
>> + * config_reg_store - Write config_reg sysfs attribute
>> + * @kobj:	Kobject structure
>> + * @attr:	Kobject attribute structure
>> + * @buf:	User entered health_status attribute string
>> + * @count:	Buffer size
>> + *
>> + * User-space interface for setting the config register.
>> + *
>> + * To write any CSU/PMU register
>> + * echo <address> <mask> <values> > /sys/firmware/zynqmp/config_reg
>> + * Usage:
>> + * echo 0x345AB234 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/config_reg
>> + *
>> + * To Read any CSU/PMU register, write address to the variable like below
>> + * echo <address> > /sys/firmware/zynqmp/config_reg
>> + *
>> + * Return:	count argument if request succeeds, the corresponding error
>> + *		code otherwise
>> + */
>> +static ssize_t config_reg_store(struct kobject *kobj,
>> +				struct kobj_attribute *attr,
>> +				const char *buf, size_t count)
>> +{
>> +	char *kern_buff, *inbuf, *tok;
>> +	unsigned long address, value, mask;
>> +	int ret;
>> +
>> +	kern_buff = kzalloc(count, GFP_KERNEL);
>> +	if (!kern_buff)
>> +		return -ENOMEM;
>> +
>> +	ret = strlcpy(kern_buff, buf, count);
>> +	if (ret < 0) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
> 
> Greg: What's the recommended way how to parse parameters via sysfs?
> I see that kstrndup is used for cloning buffer instead of kzalloc
> followed by strchr and sscanf.
> 
> 
>> +
>> +	inbuf = kern_buff;
>> +
>> +	/* Read the addess */
> 
> typo here.
> 
>> +	tok = strsep(&inbuf, " ");
>> +	if (!tok) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
>> +	ret = kstrtol(tok, 16, &address);
>> +	if (ret) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
>> +	/* Read the write value */
>> +	tok = strsep(&inbuf, " ");
>> +	/*
>> +	 * If parameter provided is only address, then its a read operation.
>> +	 * Store the address in a global variable and retrieve whenever
>> +	 * required.
>> +	 */
>> +	if (!tok) {
>> +		register_address = address;
>> +		goto err;
>> +	}
>> +	register_address = address;
>> +
>> +	ret = kstrtol(tok, 16, &mask);
>> +	if (ret) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
>> +	tok = strsep(&inbuf, " ");
>> +	if (!tok) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
>> +	ret = kstrtol(tok, 16, &value);
>> +	if (!tok) {
>> +		ret = -EFAULT;
>> +		goto err;
>> +	}
>> +	ret = zynqmp_pm_config_reg_access(CONFIG_REG_WRITE, address,
>> +					  mask, value, NULL);
>> +	if (ret)
>> +		pr_err("unable to write value to %lx\n", value);
>> +err:
>> +	kfree(kern_buff);
>> +	if (ret)
>> +		return ret;
>> +	return count;
>> +}
>> +
>> +/**
>> + * config_reg_show - Read config_reg sysfs attribute
>> + * @kobj:	Kobject structure
>> + * @attr:	Kobject attribute structure
>> + * @buf:	User entered health_status attribute string
>> + *
>> + * User-space interface for getting the config register.
>> + *
>> + * To Read any CSU/PMU register, write address to the variable like below
>> + * echo <address> > /sys/firmware/zynqmp/config_reg
>> + *
>> + * Then Read the address using below command
>> + * cat /sys/firmware/zynqmp/config_reg
>> + *
>> + * Return: number of chars written to buf.
>> + */
>> +static ssize_t config_reg_show(struct kobject *kobj,
>> +			       struct kobj_attribute *attr,
>> +			       char *buf)
>> +{
>> +	int ret;
>> +	u32 ret_payload[PAYLOAD_ARG_CNT];
>> +
>> +	ret = zynqmp_pm_config_reg_access(CONFIG_REG_READ, register_address,
>> +					  0, 0, ret_payload);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sprintf(buf, "0x%x\n", ret_payload[1]);
>> +}
>> +
>> +static struct kobj_attribute zynqmp_attr_config_reg =
>> +					__ATTR_RW(config_reg);
> 
> 
> use DEVICE_ATTR_RW() instead
> 
>> +
>> +static struct attribute *attrs[] = {
>> +	&zynqmp_attr_config_reg.attr,
>> +	NULL,
>> +};
> 
>> +
>> +static const struct attribute_group attr_group = {
>> +	.attrs = attrs,
>> +	NULL,
>> +};
> 
> ATTRIBUTE_GROUPS instead.
> 
>> +
>> +static int zynqmp_pm_sysfs_init(void)
>> +{
>> +	struct kobject *zynqmp_kobj;
>> +	int ret;
>> +
>> +	zynqmp_kobj = kobject_create_and_add("zynqmp", firmware_kobj);
>> +	if (!zynqmp_kobj) {
>> +		pr_err("zynqmp: Firmware kobj add failed.\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	ret = sysfs_create_group(zynqmp_kobj, &attr_group);
>> +	if (ret) {
>> +		pr_err("%s() sysfs creation fail with error %d\n",
>> +		       __func__, ret);
> 
> if you fail here you should free kobject_put(zynqmp_kobj);
> 
> 
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static int zynqmp_firmware_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -748,6 +925,12 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
>>  	/* Assign eemi_ops_table */
>>  	eemi_ops_tbl = &eemi_ops;
>>  
>> +	ret = zynqmp_pm_sysfs_init();
>> +	if (ret) {
>> +		pr_err("%s() sysfs init fail with error %d\n", __func__, ret);
>> +		return ret;
>> +	}
>> +
>>  	zynqmp_pm_api_debugfs_init();
>>  
>>  	ret = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, firmware_devs,
>> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
>> index df366f1..55561d0 100644
>> --- a/include/linux/firmware/xlnx-zynqmp.h
>> +++ b/include/linux/firmware/xlnx-zynqmp.h
>> @@ -77,6 +77,8 @@ enum pm_api_id {
>>  	PM_CLOCK_GETRATE,
>>  	PM_CLOCK_SETPARENT,
>>  	PM_CLOCK_GETPARENT,
>> +	/* PM_REGISTER_ACCESS API */
>> +	PM_REGISTER_ACCESS = 52,
>>  };
>>  
>>  /* PMU-FW return status codes */
>> @@ -261,6 +263,11 @@ enum tap_delay_type {
>>  	PM_TAPDELAY_OUTPUT,
>>  };
>>  
>> +enum pm_register_access_id {
>> +	CONFIG_REG_WRITE,
>> +	CONFIG_REG_READ,
>> +};
>> +
>>  /**
>>   * struct zynqmp_pm_query_data - PM query data
>>   * @qid:	query ID
>> @@ -305,6 +312,8 @@ struct zynqmp_eemi_ops {
>>  			       const u32 capabilities,
>>  			       const u32 qos,
>>  			       const enum zynqmp_pm_request_ack ack);
>> +	int (*register_access)(u32 register_access_id, u32 address,
>> +			       u32 mask, u32 value, u32 *out);
>>  };
>>  
>>  int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
>>
> 

One more thing. I am missing sysfs description for these entries.
You are creating this file in 2/5 but files introduced here are not
cover there.

Thanks,
Michal


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] firmware: xilinx: Adds new eemi call for reg access

[Sudeep Holla]

On Wed, Dec 04, 2019 at 03:29:15PM -0800, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> This patch adds new EEMI call which is used for CSU/PMU register
> access from linux.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp.c     | 183 +++++++++++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h |   9 ++
>  2 files changed, 192 insertions(+)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index fd3d837..56431ad 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -24,6 +24,8 @@
>  #include <linux/firmware/xlnx-zynqmp.h>
>  #include "zynqmp-debug.h"
>  
> +static unsigned long register_address;
> +
>  static const struct zynqmp_eemi_ops *eemi_ops_tbl;
>  
>  static const struct mfd_cell firmware_devs[] = {
> @@ -664,6 +666,26 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
>  				   qos, ack, NULL);
>  }
>  
> +/**
> + * zynqmp_pm_config_reg_access - PM Config API for Config register access
> + * @register_access_id:	ID of the requested REGISTER_ACCESS
> + * @address:		Address of the register to be accessed
> + * @mask:		Mask to be written to the register
> + * @value:		Value to be written to the register
> + * @out:		Returned output value
> + *
> + * This function calls REGISTER_ACCESS to configure CSU/PMU registers.
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +
> +static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
> +				       u32 mask, u32 value, u32 *out)
> +{
> +	return zynqmp_pm_invoke_fn(PM_REGISTER_ACCESS, register_access_id,
> +				   address, mask, value, out);
> +}
> +

If you have this API, can you remove all other APIs and implement them
using these ? This kills the EEMI abstraction.

NACK for this and any attempts to provide direct reas/write access to
the PM config space. EEMI should have better abstraction than this.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] firmware: xilinx: Add sysfs interface

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

Add Firmware-ggs sysfs interface which provides read/write
interface to global storage registers.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 Documentation/ABI/stable/sysfs-firmware-zynqmp |  50 +++++
 drivers/firmware/xilinx/Makefile               |   2 +-
 drivers/firmware/xilinx/zynqmp-ggs.c           | 289 +++++++++++++++++++++++++
 drivers/firmware/xilinx/zynqmp.c               |  12 +
 include/linux/firmware/xlnx-zynqmp.h           |  10 +
 5 files changed, 362 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-firmware-zynqmp
 create mode 100644 drivers/firmware/xilinx/zynqmp-ggs.c

diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
new file mode 100644
index 0000000..0a75812
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
@@ -0,0 +1,50 @@
+What:		/sys/firmware/zynqmp/ggs*
+Date:		January 2018
+KernelVersion:	4.15.0
+Contact:	"Jolly Shah" <jollys@xilinx.com>
+Description:
+		Read/Write PMU global general storage register value,
+		GLOBAL_GEN_STORAGE{0:3}.
+		Global general storage register that can be used
+		by system to pass information between masters.
+
+		The register is reset during system or power-on
+		resets. Three registers are used by the FSBL and
+		other Xilinx software products: GLOBAL_GEN_STORAGE{4:6}.
+
+		Usage:
+		# cat /sys/firmware/zynqmp/ggs0
+		# echo <mask> <value> > /sys/firmware/zynqmp/ggs0
+
+		Example:
+		# cat /sys/firmware/zynqmp/ggs0
+		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/ggs0
+
+Users:		Xilinx
+
+What:		/sys/firmware/zynqmp/pggs*
+Date:		January 2018
+KernelVersion:	4.15.0
+Contact:	"Jolly Shah" <jollys@xilinx.com>
+Description:
+		Read/Write PMU persistent global general storage register
+		value, PERS_GLOB_GEN_STORAGE{0:3}.
+		Persistent global general storage register that
+		can be used by system to pass information between
+		masters.
+
+		This register is only reset by the power-on reset
+		and maintains its value through a system reset.
+		Four registers are used by the FSBL and other Xilinx
+		software products: PERS_GLOB_GEN_STORAGE{4:7}.
+		Register is reset only by a POR reset.
+
+		Usage:
+		# cat /sys/firmware/zynqmp/pggs0
+		# echo <mask> <value> > /sys/firmware/zynqmp/pggs0
+
+		Example:
+		# cat /sys/firmware/zynqmp/pggs0
+		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/pggs0
+
+Users:		Xilinx
diff --git a/drivers/firmware/xilinx/Makefile b/drivers/firmware/xilinx/Makefile
index 875a537..1e8643c 100644
--- a/drivers/firmware/xilinx/Makefile
+++ b/drivers/firmware/xilinx/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for Xilinx firmwares
 
-obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o
+obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o zynqmp-ggs.o
 obj-$(CONFIG_ZYNQMP_FIRMWARE_DEBUG) += zynqmp-debug.o
diff --git a/drivers/firmware/xilinx/zynqmp-ggs.c b/drivers/firmware/xilinx/zynqmp-ggs.c
new file mode 100644
index 0000000..42179ad
--- /dev/null
+++ b/drivers/firmware/xilinx/zynqmp-ggs.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Xilinx Zynq MPSoC Firmware layer
+ *
+ *  Copyright (C) 2014-2018 Xilinx, Inc.
+ *
+ *  Jolly Shah <jollys@xilinx.com>
+ *  Rajan Vaja <rajanv@xilinx.com>
+ */
+
+#include <linux/compiler.h>
+#include <linux/of.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/slab.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+static ssize_t read_register(char *buf, u32 ioctl_id, u32 reg)
+{
+	int ret;
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops->ioctl)
+		return -EFAULT;
+
+	ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "0x%x\n", ret_payload[1]);
+}
+
+static ssize_t write_register(const char *buf, size_t count, u32 read_ioctl,
+			      u32 write_ioctl, u32 reg)
+{
+	char *kern_buff, *inbuf, *tok;
+	long mask, value;
+	int ret;
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops->ioctl)
+		return -EFAULT;
+
+	kern_buff = kzalloc(count, GFP_KERNEL);
+	if (!kern_buff)
+		return -ENOMEM;
+
+	ret = strlcpy(kern_buff, buf, count);
+	if (ret < 0) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	inbuf = kern_buff;
+
+	/* Read the write mask */
+	tok = strsep(&inbuf, " ");
+	if (!tok) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	ret = kstrtol(tok, 16, &mask);
+	if (ret) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	/* Read the write value */
+	tok = strsep(&inbuf, " ");
+	if (!tok) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	ret = kstrtol(tok, 16, &value);
+	if (ret) {
+		ret = -EFAULT;
+		goto err;
+	}
+
+	ret = eemi_ops->ioctl(0, read_ioctl, reg, 0, ret_payload);
+	if (ret) {
+		ret = -EFAULT;
+		goto err;
+	}
+	ret_payload[1] &= ~mask;
+	value &= mask;
+	value |= ret_payload[1];
+
+	ret = eemi_ops->ioctl(0, write_ioctl, reg, value, NULL);
+	if (ret)
+		ret = -EFAULT;
+
+err:
+	kfree(kern_buff);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+/**
+ * ggs_show - Show global general storage (ggs) sysfs attribute
+ * @kobj: Kobject structure
+ * @attr: Kobject attribute structure
+ * @buf: Requested available shutdown_scope attributes string
+ * @reg: Register number
+ *
+ * Return:Number of bytes printed into the buffer.
+ *
+ * Helper function for viewing a ggs register value.
+ *
+ * User-space interface for viewing the content of the ggs0 register.
+ * cat /sys/firmware/zynqmp/ggs0
+ */
+static ssize_t ggs_show(struct kobject *kobj,
+			struct kobj_attribute *attr,
+			char *buf,
+			u32 reg)
+{
+	return read_register(buf, IOCTL_READ_GGS, reg);
+}
+
+/**
+ * ggs_store - Store global general storage (ggs) sysfs attribute
+ * @kobj: Kobject structure
+ * @attr: Kobject attribute structure
+ * @buf: User entered shutdown_scope attribute string
+ * @count: Size of buf
+ * @reg: Register number
+ *
+ * Return: count argument if request succeeds, the corresponding
+ * error code otherwise
+ *
+ * Helper function for storing a ggs register value.
+ *
+ * For example, the user-space interface for storing a value to the
+ * ggs0 register:
+ * echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/ggs0
+ */
+static ssize_t ggs_store(struct kobject *kobj,
+			 struct kobj_attribute *attr,
+			 const char *buf,
+			 size_t count,
+			 u32 reg)
+{
+	if (!kobj || !attr || !buf || !count || reg >= GSS_NUM_REGS)
+		return -EINVAL;
+
+	return write_register(buf, count, IOCTL_READ_GGS, IOCTL_WRITE_GGS, reg);
+}
+
+/* GGS register show functions */
+#define GGS0_SHOW(N)						\
+	ssize_t ggs##N##_show(struct kobject *kobj,		\
+				struct kobj_attribute *attr,	\
+				char *buf)			\
+	{							\
+		return ggs_show(kobj, attr, buf, N);		\
+	}
+
+static GGS0_SHOW(0);
+static GGS0_SHOW(1);
+static GGS0_SHOW(2);
+static GGS0_SHOW(3);
+
+/* GGS register store function */
+#define GGS0_STORE(N)						\
+	ssize_t ggs##N##_store(struct kobject *kobj,		\
+				struct kobj_attribute *attr,	\
+				const char *buf,		\
+				size_t count)			\
+	{							\
+		return ggs_store(kobj, attr, buf, count, N);	\
+	}
+
+static GGS0_STORE(0);
+static GGS0_STORE(1);
+static GGS0_STORE(2);
+static GGS0_STORE(3);
+
+/**
+ * pggs_show - Show persistent global general storage (pggs) sysfs attribute
+ * @kobj: Kobject structure
+ * @attr: Kobject attribute structure
+ * @buf: Requested available shutdown_scope attributes string
+ * @reg: Register number
+ *
+ * Return:Number of bytes printed into the buffer.
+ *
+ * Helper function for viewing a pggs register value.
+ */
+static ssize_t pggs_show(struct kobject *kobj,
+			 struct kobj_attribute *attr,
+			 char *buf,
+			 u32 reg)
+{
+	return read_register(buf, IOCTL_READ_PGGS, reg);
+}
+
+/**
+ * pggs_store - Store persistent global general storage (pggs) sysfs attribute
+ * @kobj: Kobject structure
+ * @attr: Kobject attribute structure
+ * @buf: User entered shutdown_scope attribute string
+ * @count: Size of buf
+ * @reg: Register number
+ *
+ * Return: count argument if request succeeds, the corresponding
+ * error code otherwise
+ *
+ * Helper function for storing a pggs register value.
+ */
+static ssize_t pggs_store(struct kobject *kobj,
+			  struct kobj_attribute *attr,
+			  const char *buf,
+			  size_t count,
+			  u32 reg)
+{
+	return write_register(buf, count, IOCTL_READ_PGGS,
+			      IOCTL_WRITE_PGGS, reg);
+}
+
+#define PGGS0_SHOW(N)						\
+	ssize_t pggs##N##_show(struct kobject *kobj,		\
+				struct kobj_attribute *attr,	\
+				char *buf)			\
+	{							\
+		return pggs_show(kobj, attr, buf, N);		\
+	}
+
+#define PGGS0_STORE(N)						\
+	ssize_t pggs##N##_store(struct kobject *kobj,		\
+				   struct kobj_attribute *attr,	\
+				   const char *buf,		\
+				   size_t count)		\
+	{							\
+		return pggs_store(kobj, attr, buf, count, N);	\
+	}
+
+/* PGGS register show functions */
+static PGGS0_SHOW(0);
+static PGGS0_SHOW(1);
+static PGGS0_SHOW(2);
+static PGGS0_SHOW(3);
+
+/* PGGS register store functions */
+static PGGS0_STORE(0);
+static PGGS0_STORE(1);
+static PGGS0_STORE(2);
+static PGGS0_STORE(3);
+
+/* GGS register attributes */
+static struct kobj_attribute zynqmp_attr_ggs0 = __ATTR_RW(ggs0);
+static struct kobj_attribute zynqmp_attr_ggs1 = __ATTR_RW(ggs1);
+static struct kobj_attribute zynqmp_attr_ggs2 = __ATTR_RW(ggs2);
+static struct kobj_attribute zynqmp_attr_ggs3 = __ATTR_RW(ggs3);
+
+/* PGGS register attributes */
+static struct kobj_attribute zynqmp_attr_pggs0 = __ATTR_RW(pggs0);
+static struct kobj_attribute zynqmp_attr_pggs1 = __ATTR_RW(pggs1);
+static struct kobj_attribute zynqmp_attr_pggs2 = __ATTR_RW(pggs2);
+static struct kobj_attribute zynqmp_attr_pggs3 = __ATTR_RW(pggs3);
+
+static struct attribute *attrs[] = {
+	&zynqmp_attr_ggs0.attr,
+	&zynqmp_attr_ggs1.attr,
+	&zynqmp_attr_ggs2.attr,
+	&zynqmp_attr_ggs3.attr,
+	&zynqmp_attr_pggs0.attr,
+	&zynqmp_attr_pggs1.attr,
+	&zynqmp_attr_pggs2.attr,
+	&zynqmp_attr_pggs3.attr,
+	NULL,
+};
+
+static const struct attribute_group attr_group = {
+	.attrs = attrs,
+	NULL,
+};
+
+int zynqmp_pm_ggs_init(struct kobject *parent_kobj)
+{
+	return sysfs_create_group(parent_kobj, &attr_group);
+}
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 56431ad..9836174 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -475,6 +475,10 @@ static inline int zynqmp_is_valid_ioctl(u32 ioctl_id)
 	case IOCTL_GET_PLL_FRAC_MODE:
 	case IOCTL_SET_PLL_FRAC_DATA:
 	case IOCTL_GET_PLL_FRAC_DATA:
+	case IOCTL_WRITE_GGS:
+	case IOCTL_READ_GGS:
+	case IOCTL_WRITE_PGGS:
+	case IOCTL_READ_PGGS:
 		return 1;
 	default:
 		return 0;
@@ -876,8 +880,16 @@ static int zynqmp_pm_sysfs_init(void)
 	if (ret) {
 		pr_err("%s() sysfs creation fail with error %d\n",
 		       __func__, ret);
+		goto err;
 	}
 
+	ret = zynqmp_pm_ggs_init(zynqmp_kobj);
+	if (ret) {
+		pr_err("%s() GGS init fail with error %d\n",
+		       __func__, ret);
+		goto err;
+	}
+err:
 	return ret;
 }
 
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 55561d0..bf6e9db 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -13,6 +13,8 @@
 #ifndef __FIRMWARE_ZYNQMP_H__
 #define __FIRMWARE_ZYNQMP_H__
 
+#include <linux/device.h>
+
 #define ZYNQMP_PM_VERSION_MAJOR	1
 #define ZYNQMP_PM_VERSION_MINOR	0
 
@@ -42,6 +44,8 @@
 
 #define ZYNQMP_PM_MAX_QOS		100U
 
+#define GSS_NUM_REGS	(4)
+
 /* Node capabilities */
 #define	ZYNQMP_PM_CAPABILITY_ACCESS	0x1U
 #define	ZYNQMP_PM_CAPABILITY_CONTEXT	0x2U
@@ -98,6 +102,10 @@ enum pm_ioctl_id {
 	IOCTL_GET_PLL_FRAC_MODE,
 	IOCTL_SET_PLL_FRAC_DATA,
 	IOCTL_GET_PLL_FRAC_DATA,
+	IOCTL_WRITE_GGS,
+	IOCTL_READ_GGS,
+	IOCTL_WRITE_PGGS,
+	IOCTL_READ_PGGS,
 };
 
 enum pm_query_id {
@@ -319,6 +327,8 @@ struct zynqmp_eemi_ops {
 int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
 			u32 arg2, u32 arg3, u32 *ret_payload);
 
+int zynqmp_pm_ggs_init(struct kobject *parent_kobj);
+
 #if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
 const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void);
 #else
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/5] firmware: xilinx: Add sysfs interface

[Michal Simek]

On 05. 12. 19 0:29, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Add Firmware-ggs sysfs interface which provides read/write
> interface to global storage registers.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  Documentation/ABI/stable/sysfs-firmware-zynqmp |  50 +++++
>  drivers/firmware/xilinx/Makefile               |   2 +-
>  drivers/firmware/xilinx/zynqmp-ggs.c           | 289 +++++++++++++++++++++++++
>  drivers/firmware/xilinx/zynqmp.c               |  12 +
>  include/linux/firmware/xlnx-zynqmp.h           |  10 +
>  5 files changed, 362 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/stable/sysfs-firmware-zynqmp
>  create mode 100644 drivers/firmware/xilinx/zynqmp-ggs.c
> 
> diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> new file mode 100644
> index 0000000..0a75812
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> @@ -0,0 +1,50 @@
> +What:		/sys/firmware/zynqmp/ggs*
> +Date:		January 2018
> +KernelVersion:	4.15.0

looks old.

> +Contact:	"Jolly Shah" <jollys@xilinx.com>
> +Description:
> +		Read/Write PMU global general storage register value,
> +		GLOBAL_GEN_STORAGE{0:3}.
> +		Global general storage register that can be used
> +		by system to pass information between masters.
> +
> +		The register is reset during system or power-on
> +		resets. Three registers are used by the FSBL and
> +		other Xilinx software products: GLOBAL_GEN_STORAGE{4:6}.
> +
> +		Usage:
> +		# cat /sys/firmware/zynqmp/ggs0
> +		# echo <mask> <value> > /sys/firmware/zynqmp/ggs0
> +
> +		Example:
> +		# cat /sys/firmware/zynqmp/ggs0
> +		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/ggs0
> +
> +Users:		Xilinx
> +
> +What:		/sys/firmware/zynqmp/pggs*
> +Date:		January 2018
> +KernelVersion:	4.15.0

ditto.

> +Contact:	"Jolly Shah" <jollys@xilinx.com>
> +Description:
> +		Read/Write PMU persistent global general storage register
> +		value, PERS_GLOB_GEN_STORAGE{0:3}.
> +		Persistent global general storage register that
> +		can be used by system to pass information between
> +		masters.
> +
> +		This register is only reset by the power-on reset
> +		and maintains its value through a system reset.
> +		Four registers are used by the FSBL and other Xilinx
> +		software products: PERS_GLOB_GEN_STORAGE{4:7}.
> +		Register is reset only by a POR reset.
> +
> +		Usage:
> +		# cat /sys/firmware/zynqmp/pggs0
> +		# echo <mask> <value> > /sys/firmware/zynqmp/pggs0
> +
> +		Example:
> +		# cat /sys/firmware/zynqmp/pggs0
> +		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/pggs0
> +
> +Users:		Xilinx
> diff --git a/drivers/firmware/xilinx/Makefile b/drivers/firmware/xilinx/Makefile
> index 875a537..1e8643c 100644
> --- a/drivers/firmware/xilinx/Makefile
> +++ b/drivers/firmware/xilinx/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for Xilinx firmwares
>  
> -obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o
> +obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o zynqmp-ggs.o
>  obj-$(CONFIG_ZYNQMP_FIRMWARE_DEBUG) += zynqmp-debug.o
> diff --git a/drivers/firmware/xilinx/zynqmp-ggs.c b/drivers/firmware/xilinx/zynqmp-ggs.c
> new file mode 100644
> index 0000000..42179ad
> --- /dev/null
> +++ b/drivers/firmware/xilinx/zynqmp-ggs.c
> @@ -0,0 +1,289 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Xilinx Zynq MPSoC Firmware layer
> + *
> + *  Copyright (C) 2014-2018 Xilinx, Inc.
> + *
> + *  Jolly Shah <jollys@xilinx.com>
> + *  Rajan Vaja <rajanv@xilinx.com>
> + */
> +
> +#include <linux/compiler.h>
> +#include <linux/of.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +
> +static ssize_t read_register(char *buf, u32 ioctl_id, u32 reg)
> +{
> +	int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
> +
> +	if (!eemi_ops->ioctl)
> +		return -EFAULT;
> +
> +	ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "0x%x\n", ret_payload[1]);
> +}
> +
> +static ssize_t write_register(const char *buf, size_t count, u32 read_ioctl,
> +			      u32 write_ioctl, u32 reg)
> +{
> +	char *kern_buff, *inbuf, *tok;
> +	long mask, value;
> +	int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
> +
> +	if (!eemi_ops->ioctl)
> +		return -EFAULT;
> +
> +	kern_buff = kzalloc(count, GFP_KERNEL);
> +	if (!kern_buff)
> +		return -ENOMEM;
> +
> +	ret = strlcpy(kern_buff, buf, count);
> +	if (ret < 0) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	inbuf = kern_buff;
> +
> +	/* Read the write mask */
> +	tok = strsep(&inbuf, " ");
> +	if (!tok) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	ret = kstrtol(tok, 16, &mask);
> +	if (ret) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	/* Read the write value */
> +	tok = strsep(&inbuf, " ");
> +	if (!tok) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	ret = kstrtol(tok, 16, &value);
> +	if (ret) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +
> +	ret = eemi_ops->ioctl(0, read_ioctl, reg, 0, ret_payload);
> +	if (ret) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	ret_payload[1] &= ~mask;
> +	value &= mask;
> +	value |= ret_payload[1];
> +
> +	ret = eemi_ops->ioctl(0, write_ioctl, reg, value, NULL);
> +	if (ret)
> +		ret = -EFAULT;
> +
> +err:
> +	kfree(kern_buff);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +/**
> + * ggs_show - Show global general storage (ggs) sysfs attribute
> + * @kobj: Kobject structure
> + * @attr: Kobject attribute structure
> + * @buf: Requested available shutdown_scope attributes string
> + * @reg: Register number
> + *
> + * Return:Number of bytes printed into the buffer.
> + *
> + * Helper function for viewing a ggs register value.
> + *
> + * User-space interface for viewing the content of the ggs0 register.
> + * cat /sys/firmware/zynqmp/ggs0
> + */
> +static ssize_t ggs_show(struct kobject *kobj,
> +			struct kobj_attribute *attr,
> +			char *buf,
> +			u32 reg)
> +{
> +	return read_register(buf, IOCTL_READ_GGS, reg);
> +}
> +
> +/**
> + * ggs_store - Store global general storage (ggs) sysfs attribute
> + * @kobj: Kobject structure
> + * @attr: Kobject attribute structure
> + * @buf: User entered shutdown_scope attribute string
> + * @count: Size of buf
> + * @reg: Register number
> + *
> + * Return: count argument if request succeeds, the corresponding
> + * error code otherwise
> + *
> + * Helper function for storing a ggs register value.
> + *
> + * For example, the user-space interface for storing a value to the
> + * ggs0 register:
> + * echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/ggs0
> + */
> +static ssize_t ggs_store(struct kobject *kobj,
> +			 struct kobj_attribute *attr,
> +			 const char *buf,
> +			 size_t count,
> +			 u32 reg)
> +{
> +	if (!kobj || !attr || !buf || !count || reg >= GSS_NUM_REGS)
> +		return -EINVAL;
> +
> +	return write_register(buf, count, IOCTL_READ_GGS, IOCTL_WRITE_GGS, reg);
> +}
> +
> +/* GGS register show functions */
> +#define GGS0_SHOW(N)						\
> +	ssize_t ggs##N##_show(struct kobject *kobj,		\
> +				struct kobj_attribute *attr,	\
> +				char *buf)			\
> +	{							\
> +		return ggs_show(kobj, attr, buf, N);		\
> +	}
> +
> +static GGS0_SHOW(0);
> +static GGS0_SHOW(1);
> +static GGS0_SHOW(2);
> +static GGS0_SHOW(3);
> +
> +/* GGS register store function */
> +#define GGS0_STORE(N)						\
> +	ssize_t ggs##N##_store(struct kobject *kobj,		\
> +				struct kobj_attribute *attr,	\
> +				const char *buf,		\
> +				size_t count)			\
> +	{							\
> +		return ggs_store(kobj, attr, buf, count, N);	\
> +	}
> +
> +static GGS0_STORE(0);
> +static GGS0_STORE(1);
> +static GGS0_STORE(2);
> +static GGS0_STORE(3);
> +
> +/**
> + * pggs_show - Show persistent global general storage (pggs) sysfs attribute
> + * @kobj: Kobject structure
> + * @attr: Kobject attribute structure
> + * @buf: Requested available shutdown_scope attributes string
> + * @reg: Register number
> + *
> + * Return:Number of bytes printed into the buffer.
> + *
> + * Helper function for viewing a pggs register value.
> + */
> +static ssize_t pggs_show(struct kobject *kobj,
> +			 struct kobj_attribute *attr,
> +			 char *buf,
> +			 u32 reg)
> +{
> +	return read_register(buf, IOCTL_READ_PGGS, reg);
> +}
> +
> +/**
> + * pggs_store - Store persistent global general storage (pggs) sysfs attribute
> + * @kobj: Kobject structure
> + * @attr: Kobject attribute structure
> + * @buf: User entered shutdown_scope attribute string
> + * @count: Size of buf
> + * @reg: Register number
> + *
> + * Return: count argument if request succeeds, the corresponding
> + * error code otherwise
> + *
> + * Helper function for storing a pggs register value.
> + */
> +static ssize_t pggs_store(struct kobject *kobj,
> +			  struct kobj_attribute *attr,
> +			  const char *buf,
> +			  size_t count,
> +			  u32 reg)
> +{
> +	return write_register(buf, count, IOCTL_READ_PGGS,
> +			      IOCTL_WRITE_PGGS, reg);
> +}
> +
> +#define PGGS0_SHOW(N)						\
> +	ssize_t pggs##N##_show(struct kobject *kobj,		\
> +				struct kobj_attribute *attr,	\
> +				char *buf)			\
> +	{							\
> +		return pggs_show(kobj, attr, buf, N);		\
> +	}
> +
> +#define PGGS0_STORE(N)						\
> +	ssize_t pggs##N##_store(struct kobject *kobj,		\
> +				   struct kobj_attribute *attr,	\
> +				   const char *buf,		\
> +				   size_t count)		\
> +	{							\
> +		return pggs_store(kobj, attr, buf, count, N);	\
> +	}
> +
> +/* PGGS register show functions */
> +static PGGS0_SHOW(0);
> +static PGGS0_SHOW(1);
> +static PGGS0_SHOW(2);
> +static PGGS0_SHOW(3);
> +
> +/* PGGS register store functions */
> +static PGGS0_STORE(0);
> +static PGGS0_STORE(1);
> +static PGGS0_STORE(2);
> +static PGGS0_STORE(3);
> +
> +/* GGS register attributes */
> +static struct kobj_attribute zynqmp_attr_ggs0 = __ATTR_RW(ggs0);
> +static struct kobj_attribute zynqmp_attr_ggs1 = __ATTR_RW(ggs1);
> +static struct kobj_attribute zynqmp_attr_ggs2 = __ATTR_RW(ggs2);
> +static struct kobj_attribute zynqmp_attr_ggs3 = __ATTR_RW(ggs3);
> +
> +/* PGGS register attributes */
> +static struct kobj_attribute zynqmp_attr_pggs0 = __ATTR_RW(pggs0);
> +static struct kobj_attribute zynqmp_attr_pggs1 = __ATTR_RW(pggs1);
> +static struct kobj_attribute zynqmp_attr_pggs2 = __ATTR_RW(pggs2);
> +static struct kobj_attribute zynqmp_attr_pggs3 = __ATTR_RW(pggs3);
> +
> +static struct attribute *attrs[] = {
> +	&zynqmp_attr_ggs0.attr,
> +	&zynqmp_attr_ggs1.attr,
> +	&zynqmp_attr_ggs2.attr,
> +	&zynqmp_attr_ggs3.attr,
> +	&zynqmp_attr_pggs0.attr,
> +	&zynqmp_attr_pggs1.attr,
> +	&zynqmp_attr_pggs2.attr,
> +	&zynqmp_attr_pggs3.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group attr_group = {
> +	.attrs = attrs,
> +	NULL,
> +};

the same as was in 1/5

> +
> +int zynqmp_pm_ggs_init(struct kobject *parent_kobj)
> +{
> +	return sysfs_create_group(parent_kobj, &attr_group);
> +}
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 56431ad..9836174 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -475,6 +475,10 @@ static inline int zynqmp_is_valid_ioctl(u32 ioctl_id)
>  	case IOCTL_GET_PLL_FRAC_MODE:
>  	case IOCTL_SET_PLL_FRAC_DATA:
>  	case IOCTL_GET_PLL_FRAC_DATA:
> +	case IOCTL_WRITE_GGS:
> +	case IOCTL_READ_GGS:
> +	case IOCTL_WRITE_PGGS:
> +	case IOCTL_READ_PGGS:
>  		return 1;
>  	default:
>  		return 0;
> @@ -876,8 +880,16 @@ static int zynqmp_pm_sysfs_init(void)
>  	if (ret) {
>  		pr_err("%s() sysfs creation fail with error %d\n",
>  		       __func__, ret);
> +		goto err;
>  	}
>  
> +	ret = zynqmp_pm_ggs_init(zynqmp_kobj);
> +	if (ret) {
> +		pr_err("%s() GGS init fail with error %d\n",
> +		       __func__, ret);
> +		goto err;
> +	}
> +err:
>  	return ret;
>  }
>  
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 55561d0..bf6e9db 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -13,6 +13,8 @@
>  #ifndef __FIRMWARE_ZYNQMP_H__
>  #define __FIRMWARE_ZYNQMP_H__
>  
> +#include <linux/device.h>
> +
>  #define ZYNQMP_PM_VERSION_MAJOR	1
>  #define ZYNQMP_PM_VERSION_MINOR	0
>  
> @@ -42,6 +44,8 @@
>  
>  #define ZYNQMP_PM_MAX_QOS		100U
>  
> +#define GSS_NUM_REGS	(4)
> +
>  /* Node capabilities */
>  #define	ZYNQMP_PM_CAPABILITY_ACCESS	0x1U
>  #define	ZYNQMP_PM_CAPABILITY_CONTEXT	0x2U
> @@ -98,6 +102,10 @@ enum pm_ioctl_id {
>  	IOCTL_GET_PLL_FRAC_MODE,
>  	IOCTL_SET_PLL_FRAC_DATA,
>  	IOCTL_GET_PLL_FRAC_DATA,
> +	IOCTL_WRITE_GGS,
> +	IOCTL_READ_GGS,
> +	IOCTL_WRITE_PGGS,
> +	IOCTL_READ_PGGS,
>  };
>  
>  enum pm_query_id {
> @@ -319,6 +327,8 @@ struct zynqmp_eemi_ops {
>  int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
>  			u32 arg2, u32 arg3, u32 *ret_payload);
>  
> +int zynqmp_pm_ggs_init(struct kobject *parent_kobj);
> +
>  #if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
>  const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void);
>  #else
> 

M


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] firmware: xilinx: Add system shutdown API interface

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

Add system shutdown API interface which asks firmware to
perform system shutdown/restart.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 drivers/firmware/xilinx/zynqmp.c     | 14 ++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h |  4 +++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 9836174..8dcf5a4 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -690,6 +690,19 @@ static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
 				   address, mask, value, out);
 }
 
+/**
+ * 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
+ *
+ * Return:	Returns status, either success or error+reason
+ */
+static int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype)
+{
+	return zynqmp_pm_invoke_fn(PM_SYSTEM_SHUTDOWN, type, subtype,
+				   0, 0, NULL);
+}
+
 static const struct zynqmp_eemi_ops eemi_ops = {
 	.get_api_version = zynqmp_pm_get_api_version,
 	.get_chipid = zynqmp_pm_get_chipid,
@@ -714,6 +727,7 @@ static const struct zynqmp_eemi_ops eemi_ops = {
 	.fpga_load = zynqmp_pm_fpga_load,
 	.fpga_get_status = zynqmp_pm_fpga_get_status,
 	.register_access = zynqmp_pm_config_reg_access,
+	.system_shutdown = zynqmp_pm_system_shutdown,
 };
 
 /**
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index bf6e9db..b96ea5d 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -61,7 +61,8 @@
 
 enum pm_api_id {
 	PM_GET_API_VERSION = 1,
-	PM_REQUEST_NODE = 13,
+	PM_SYSTEM_SHUTDOWN = 12,
+	PM_REQUEST_NODE,
 	PM_RELEASE_NODE,
 	PM_SET_REQUIREMENT,
 	PM_RESET_ASSERT = 17,
@@ -322,6 +323,7 @@ struct zynqmp_eemi_ops {
 			       const enum zynqmp_pm_request_ack ack);
 	int (*register_access)(u32 register_access_id, u32 address,
 			       u32 mask, u32 value, u32 *out);
+	int (*system_shutdown)(const u32 type, const u32 subtype);
 };
 
 int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/5] firmware: xilinx: Add system shutdown API interface

[Michal Simek]

On 05. 12. 19 0:29, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Add system shutdown API interface which asks firmware to
> perform system shutdown/restart.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  drivers/firmware/xilinx/zynqmp.c     | 14 ++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h |  4 +++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 9836174..8dcf5a4 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -690,6 +690,19 @@ static int zynqmp_pm_config_reg_access(u32 register_access_id, u32 address,
>  				   address, mask, value, out);
>  }
>  
> +/**
> + * 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
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype)
> +{
> +	return zynqmp_pm_invoke_fn(PM_SYSTEM_SHUTDOWN, type, subtype,
> +				   0, 0, NULL);
> +}
> +
>  static const struct zynqmp_eemi_ops eemi_ops = {
>  	.get_api_version = zynqmp_pm_get_api_version,
>  	.get_chipid = zynqmp_pm_get_chipid,
> @@ -714,6 +727,7 @@ static const struct zynqmp_eemi_ops eemi_ops = {
>  	.fpga_load = zynqmp_pm_fpga_load,
>  	.fpga_get_status = zynqmp_pm_fpga_get_status,
>  	.register_access = zynqmp_pm_config_reg_access,
> +	.system_shutdown = zynqmp_pm_system_shutdown,
>  };
>  
>  /**
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index bf6e9db..b96ea5d 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -61,7 +61,8 @@
>  
>  enum pm_api_id {
>  	PM_GET_API_VERSION = 1,
> -	PM_REQUEST_NODE = 13,
> +	PM_SYSTEM_SHUTDOWN = 12,
> +	PM_REQUEST_NODE,
>  	PM_RELEASE_NODE,
>  	PM_SET_REQUIREMENT,
>  	PM_RESET_ASSERT = 17,
> @@ -322,6 +323,7 @@ struct zynqmp_eemi_ops {
>  			       const enum zynqmp_pm_request_ack ack);
>  	int (*register_access)(u32 register_access_id, u32 address,
>  			       u32 mask, u32 value, u32 *out);
> +	int (*system_shutdown)(const u32 type, const u32 subtype);
>  };
>  
>  int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
> 

This looks good to me.
Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] firmware: xilinx: Add sysfs to set shutdown scope

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

The Linux shutdown functionality implemented via PSCI system_off does
not include an option to set a scope, i.e. which parts of the system to
shut down.

This patch creates sysfs that allows to set the shutdown scope for the
next shutdown request. When the next shutdown is performed, the platform
specific portion of PSCI-system_off can use the chosen shutdown scope.

Signed-off-by: Stefan Krsmanovic <stefan.krsmanovic@aggios.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 Documentation/ABI/stable/sysfs-firmware-zynqmp |  32 ++++++
 drivers/firmware/xilinx/zynqmp.c               | 141 +++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h           |  12 +++
 3 files changed, 185 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
index 0a75812..13e4fd2 100644
--- a/Documentation/ABI/stable/sysfs-firmware-zynqmp
+++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
@@ -48,3 +48,35 @@ Description:
 		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/pggs0
 
 Users:		Xilinx
+
+What:		/sys/firmware/zynqmp/shutdown_scope
+Date:		February 2018
+KernelVersion:	4.15.6
+Contact:	"Jolly Shah" <jollys@xilinx.com>
+Description:
+		This sysfs interface allows to set the shutdown scope for the
+		next shutdown request. When the next shutdown is performed, the
+		platform specific portion of PSCI-system_off can use the chosen
+		shutdown scope.
+
+		Following are available shutdown scopes(subtypes):
+
+		subsystem:	Only the APU along with all of its peripherals
+				not used by other processing units will be
+				shut down. This may result in the FPD power
+				domain being shut down provided that no other
+				processing unit uses FPD peripherals or DRAM.
+		ps_only:	The complete PS will be shut down, including the
+				RPU, PMU, etc.  Only the PL domain (FPGA)
+				remains untouched.
+		system:		The complete system/device is shut down.
+
+		Usage:
+		# cat /sys/firmware/zynqmp/shutdown_scope
+		# echo <scope> > /sys/firmware/zynqmp/shutdown_scope
+
+		Example:
+		# cat /sys/firmware/zynqmp/shutdown_scope
+		# echo "subsystem" > /sys/firmware/zynqmp/shutdown_scope
+
+Users:		Xilinx
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 8dcf5a4..b23bda4 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -746,6 +746,146 @@ const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
 EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
 
 /**
+ * struct zynqmp_pm_shutdown_scope - Struct for shutdown scope
+ * @subtype:	Shutdown subtype
+ * @name:	Matching string for scope argument
+ *
+ * This struct encapsulates mapping between shutdown scope ID and string.
+ */
+struct zynqmp_pm_shutdown_scope {
+	const enum zynqmp_pm_shutdown_subtype subtype;
+	const char *name;
+};
+
+static struct zynqmp_pm_shutdown_scope shutdown_scopes[] = {
+	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM] = {
+		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM,
+		.name = "subsystem",
+	},
+	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY] = {
+		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY,
+		.name = "ps_only",
+	},
+	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM] = {
+		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM,
+		.name = "system",
+	},
+};
+
+static struct zynqmp_pm_shutdown_scope *selected_scope =
+		&shutdown_scopes[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM];
+
+/**
+ * zynqmp_pm_is_shutdown_scope_valid - Check if shutdown scope string is valid
+ * @scope_string:	Shutdown scope string
+ *
+ * Return:		Return pointer to matching shutdown scope struct from
+ *			array of available options in system if string is valid,
+ *			otherwise returns NULL.
+ */
+static struct zynqmp_pm_shutdown_scope*
+		zynqmp_pm_is_shutdown_scope_valid(const char *scope_string)
+{
+	int count;
+
+	for (count = 0; count < ARRAY_SIZE(shutdown_scopes); count++)
+		if (sysfs_streq(scope_string, shutdown_scopes[count].name))
+			return &shutdown_scopes[count];
+
+	return NULL;
+}
+
+/**
+ * shutdown_scope_show - Show shutdown_scope sysfs attribute
+ * @kobj:	Kobject structure
+ * @attr:	Kobject attribute structure
+ * @buf:	Requested available shutdown_scope attributes string
+ *
+ * User-space interface for viewing the available scope options for system
+ * shutdown. Scope option for next shutdown call is marked with [].
+ *
+ * Usage: cat /sys/firmware/zynqmp/shutdown_scope
+ *
+ * Return:	Number of bytes printed into the buffer.
+ */
+static ssize_t shutdown_scope_show(struct kobject *kobj,
+				   struct kobj_attribute *attr,
+				   char *buf)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(shutdown_scopes); i++) {
+		if (&shutdown_scopes[i] == selected_scope) {
+			strcat(buf, "[");
+			strcat(buf, shutdown_scopes[i].name);
+			strcat(buf, "]");
+		} else {
+			strcat(buf, shutdown_scopes[i].name);
+		}
+		strcat(buf, " ");
+	}
+	strcat(buf, "\n");
+
+	return strlen(buf);
+}
+
+/**
+ * shutdown_scope_store - Store shutdown_scope sysfs attribute
+ * @kobj:	Kobject structure
+ * @attr:	Kobject attribute structure
+ * @buf:	User entered shutdown_scope attribute string
+ * @count:	Buffer size
+ *
+ * User-space interface for setting the scope for the next system shutdown.
+ * Usage: echo <scope> > /sys/firmware/zynqmp/shutdown_scope
+ *
+ * The Linux shutdown functionality implemented via PSCI system_off does not
+ * include an option to set a scope, i.e. which parts of the system to shut
+ * down.
+ *
+ * This API function allows to set the shutdown scope for the next shutdown
+ * request by passing it to the ATF running in EL3. When the next shutdown
+ * is performed, the platform specific portion of PSCI-system_off can use
+ * the chosen shutdown scope.
+ *
+ * subsystem:	Only the APU along with all of its peripherals not used by other
+ *		processing units will be shut down. This may result in the FPD
+ *		power domain being shut down provided that no other processing
+ *		unit uses FPD peripherals or DRAM.
+ * ps_only:	The complete PS will be shut down, including the RPU, PMU, etc.
+ *		Only the PL domain (FPGA) remains untouched.
+ * system:	The complete system/device is shut down.
+ *
+ * Return:	count argument if request succeeds, the corresponding error
+ *		code otherwise
+ */
+static ssize_t shutdown_scope_store(struct kobject *kobj,
+				    struct kobj_attribute *attr,
+				    const char *buf, size_t count)
+{
+	int ret;
+	struct zynqmp_pm_shutdown_scope *scope;
+
+	scope = zynqmp_pm_is_shutdown_scope_valid(buf);
+	if (!scope)
+		return -EINVAL;
+
+	ret = zynqmp_pm_system_shutdown(ZYNQMP_PM_SHUTDOWN_TYPE_SETSCOPE_ONLY,
+					scope->subtype);
+	if (ret) {
+		pr_err("unable to set shutdown scope %s\n", buf);
+		return ret;
+	}
+
+	selected_scope = scope;
+
+	return count;
+}
+
+static struct kobj_attribute zynqmp_attr_shutdown_scope =
+						__ATTR_RW(shutdown_scope);
+
+/**
  * config_reg_store - Write config_reg sysfs attribute
  * @kobj:	Kobject structure
  * @attr:	Kobject attribute structure
@@ -870,6 +1010,7 @@ static struct kobj_attribute zynqmp_attr_config_reg =
 					__ATTR_RW(config_reg);
 
 static struct attribute *attrs[] = {
+	&zynqmp_attr_shutdown_scope.attr,
 	&zynqmp_attr_config_reg.attr,
 	NULL,
 };
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index b96ea5d..54061de 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -277,6 +277,18 @@ enum pm_register_access_id {
 	CONFIG_REG_READ,
 };
 
+enum zynqmp_pm_shutdown_type {
+	ZYNQMP_PM_SHUTDOWN_TYPE_SHUTDOWN,
+	ZYNQMP_PM_SHUTDOWN_TYPE_RESET,
+	ZYNQMP_PM_SHUTDOWN_TYPE_SETSCOPE_ONLY,
+};
+
+enum zynqmp_pm_shutdown_subtype {
+	ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM,
+	ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY,
+	ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM,
+};
+
 /**
  * struct zynqmp_pm_query_data - PM query data
  * @qid:	query ID
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] firmware: xilinx: Add sysfs to set shutdown scope

[Michal Simek]

On 05. 12. 19 0:29, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> The Linux shutdown functionality implemented via PSCI system_off does
> not include an option to set a scope, i.e. which parts of the system to
> shut down.
> 
> This patch creates sysfs that allows to set the shutdown scope for the
> next shutdown request. When the next shutdown is performed, the platform
> specific portion of PSCI-system_off can use the chosen shutdown scope.
> 
> Signed-off-by: Stefan Krsmanovic <stefan.krsmanovic@aggios.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  Documentation/ABI/stable/sysfs-firmware-zynqmp |  32 ++++++
>  drivers/firmware/xilinx/zynqmp.c               | 141 +++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h           |  12 +++
>  3 files changed, 185 insertions(+)
> 
> diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> index 0a75812..13e4fd2 100644
> --- a/Documentation/ABI/stable/sysfs-firmware-zynqmp
> +++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> @@ -48,3 +48,35 @@ Description:
>  		# echo 0xFFFFFFFF 0x1234ABCD > /sys/firmware/zynqmp/pggs0
>  
>  Users:		Xilinx
> +
> +What:		/sys/firmware/zynqmp/shutdown_scope
> +Date:		February 2018
> +KernelVersion:	4.15.6

ditto.

> +Contact:	"Jolly Shah" <jollys@xilinx.com>
> +Description:
> +		This sysfs interface allows to set the shutdown scope for the
> +		next shutdown request. When the next shutdown is performed, the
> +		platform specific portion of PSCI-system_off can use the chosen
> +		shutdown scope.
> +
> +		Following are available shutdown scopes(subtypes):
> +
> +		subsystem:	Only the APU along with all of its peripherals
> +				not used by other processing units will be
> +				shut down. This may result in the FPD power
> +				domain being shut down provided that no other
> +				processing unit uses FPD peripherals or DRAM.
> +		ps_only:	The complete PS will be shut down, including the
> +				RPU, PMU, etc.  Only the PL domain (FPGA)
> +				remains untouched.
> +		system:		The complete system/device is shut down.
> +
> +		Usage:
> +		# cat /sys/firmware/zynqmp/shutdown_scope
> +		# echo <scope> > /sys/firmware/zynqmp/shutdown_scope
> +
> +		Example:
> +		# cat /sys/firmware/zynqmp/shutdown_scope
> +		# echo "subsystem" > /sys/firmware/zynqmp/shutdown_scope
> +
> +Users:		Xilinx
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index 8dcf5a4..b23bda4 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -746,6 +746,146 @@ const struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
>  EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
>  
>  /**
> + * struct zynqmp_pm_shutdown_scope - Struct for shutdown scope
> + * @subtype:	Shutdown subtype
> + * @name:	Matching string for scope argument
> + *
> + * This struct encapsulates mapping between shutdown scope ID and string.
> + */
> +struct zynqmp_pm_shutdown_scope {
> +	const enum zynqmp_pm_shutdown_subtype subtype;
> +	const char *name;
> +};
> +
> +static struct zynqmp_pm_shutdown_scope shutdown_scopes[] = {
> +	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM] = {
> +		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM,
> +		.name = "subsystem",
> +	},
> +	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY] = {
> +		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY,
> +		.name = "ps_only",
> +	},
> +	[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM] = {
> +		.subtype = ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM,
> +		.name = "system",
> +	},
> +};
> +
> +static struct zynqmp_pm_shutdown_scope *selected_scope =
> +		&shutdown_scopes[ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM];
> +
> +/**
> + * zynqmp_pm_is_shutdown_scope_valid - Check if shutdown scope string is valid
> + * @scope_string:	Shutdown scope string
> + *
> + * Return:		Return pointer to matching shutdown scope struct from
> + *			array of available options in system if string is valid,
> + *			otherwise returns NULL.
> + */
> +static struct zynqmp_pm_shutdown_scope*
> +		zynqmp_pm_is_shutdown_scope_valid(const char *scope_string)
> +{
> +	int count;
> +
> +	for (count = 0; count < ARRAY_SIZE(shutdown_scopes); count++)
> +		if (sysfs_streq(scope_string, shutdown_scopes[count].name))
> +			return &shutdown_scopes[count];
> +
> +	return NULL;
> +}
> +
> +/**
> + * shutdown_scope_show - Show shutdown_scope sysfs attribute
> + * @kobj:	Kobject structure
> + * @attr:	Kobject attribute structure
> + * @buf:	Requested available shutdown_scope attributes string
> + *
> + * User-space interface for viewing the available scope options for system
> + * shutdown. Scope option for next shutdown call is marked with [].
> + *
> + * Usage: cat /sys/firmware/zynqmp/shutdown_scope
> + *
> + * Return:	Number of bytes printed into the buffer.
> + */
> +static ssize_t shutdown_scope_show(struct kobject *kobj,
> +				   struct kobj_attribute *attr,
> +				   char *buf)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(shutdown_scopes); i++) {
> +		if (&shutdown_scopes[i] == selected_scope) {
> +			strcat(buf, "[");
> +			strcat(buf, shutdown_scopes[i].name);
> +			strcat(buf, "]");
> +		} else {
> +			strcat(buf, shutdown_scopes[i].name);
> +		}
> +		strcat(buf, " ");
> +	}
> +	strcat(buf, "\n");
> +
> +	return strlen(buf);
> +}
> +
> +/**
> + * shutdown_scope_store - Store shutdown_scope sysfs attribute
> + * @kobj:	Kobject structure
> + * @attr:	Kobject attribute structure
> + * @buf:	User entered shutdown_scope attribute string
> + * @count:	Buffer size
> + *
> + * User-space interface for setting the scope for the next system shutdown.
> + * Usage: echo <scope> > /sys/firmware/zynqmp/shutdown_scope
> + *
> + * The Linux shutdown functionality implemented via PSCI system_off does not
> + * include an option to set a scope, i.e. which parts of the system to shut
> + * down.
> + *
> + * This API function allows to set the shutdown scope for the next shutdown
> + * request by passing it to the ATF running in EL3. When the next shutdown
> + * is performed, the platform specific portion of PSCI-system_off can use
> + * the chosen shutdown scope.
> + *
> + * subsystem:	Only the APU along with all of its peripherals not used by other
> + *		processing units will be shut down. This may result in the FPD
> + *		power domain being shut down provided that no other processing
> + *		unit uses FPD peripherals or DRAM.
> + * ps_only:	The complete PS will be shut down, including the RPU, PMU, etc.
> + *		Only the PL domain (FPGA) remains untouched.
> + * system:	The complete system/device is shut down.
> + *
> + * Return:	count argument if request succeeds, the corresponding error
> + *		code otherwise
> + */
> +static ssize_t shutdown_scope_store(struct kobject *kobj,
> +				    struct kobj_attribute *attr,
> +				    const char *buf, size_t count)
> +{
> +	int ret;
> +	struct zynqmp_pm_shutdown_scope *scope;
> +
> +	scope = zynqmp_pm_is_shutdown_scope_valid(buf);
> +	if (!scope)
> +		return -EINVAL;
> +
> +	ret = zynqmp_pm_system_shutdown(ZYNQMP_PM_SHUTDOWN_TYPE_SETSCOPE_ONLY,
> +					scope->subtype);
> +	if (ret) {
> +		pr_err("unable to set shutdown scope %s\n", buf);
> +		return ret;
> +	}
> +
> +	selected_scope = scope;
> +
> +	return count;
> +}
> +
> +static struct kobj_attribute zynqmp_attr_shutdown_scope =
> +						__ATTR_RW(shutdown_scope);

the same as was in 1/5.

> +
> +/**
>   * config_reg_store - Write config_reg sysfs attribute
>   * @kobj:	Kobject structure
>   * @attr:	Kobject attribute structure
> @@ -870,6 +1010,7 @@ static struct kobj_attribute zynqmp_attr_config_reg =
>  					__ATTR_RW(config_reg);
>  
>  static struct attribute *attrs[] = {
> +	&zynqmp_attr_shutdown_scope.attr,
>  	&zynqmp_attr_config_reg.attr,
>  	NULL,
>  };
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index b96ea5d..54061de 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -277,6 +277,18 @@ enum pm_register_access_id {
>  	CONFIG_REG_READ,
>  };
>  
> +enum zynqmp_pm_shutdown_type {
> +	ZYNQMP_PM_SHUTDOWN_TYPE_SHUTDOWN,
> +	ZYNQMP_PM_SHUTDOWN_TYPE_RESET,
> +	ZYNQMP_PM_SHUTDOWN_TYPE_SETSCOPE_ONLY,
> +};
> +
> +enum zynqmp_pm_shutdown_subtype {
> +	ZYNQMP_PM_SHUTDOWN_SUBTYPE_SUBSYSTEM,
> +	ZYNQMP_PM_SHUTDOWN_SUBTYPE_PS_ONLY,
> +	ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM,
> +};
> +
>  /**
>   * struct zynqmp_pm_query_data - PM query data
>   * @qid:	query ID
> 

M

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] firmware: xilinx: Add sysfs and IOCTL to set boot health status

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

Add sysfs interface to set boot health status from userspace.
Add IOCTL ID used by this interface to communicate with firmware.

If PMUFW is compiled with CHECK_HEALTHY_BOOT, it will check the
healthy bit on FPD WDT expiration. If healthy bit is set by a user
application running in Linux, PMUFW will do APU only restart. If
healthy bit is not set during FPD WDT expiration, PMUFW will do
system restart.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
---
 Documentation/ABI/stable/sysfs-firmware-zynqmp | 21 +++++++++++++
 drivers/firmware/xilinx/zynqmp.c               | 42 ++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h           |  2 ++
 3 files changed, 65 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
index 13e4fd2..eeae291 100644
--- a/Documentation/ABI/stable/sysfs-firmware-zynqmp
+++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
@@ -80,3 +80,24 @@ Description:
 		# echo "subsystem" > /sys/firmware/zynqmp/shutdown_scope
 
 Users:		Xilinx
+
+What:		/sys/firmware/zynqmp/health_status
+Date:		April 2018
+KernelVersion:	4.14.0
+Contact:	"Rajan Vaja" <rajanv@xilinx.com>
+Description:
+		This sysfs interface allows to set the health status. If PMUFW
+		is compiled with CHECK_HEALTHY_BOOT, it will check the healthy
+		bit on FPD WDT expiration. If healthy bit is set by a user
+		application running in Linux, PMUFW will do APU only restart. If
+		healthy bit is not set during FPD WDT expiration, PMUFW will do
+		system restart.
+
+		Usage:
+		Set healty bit
+		# echo 1 > /sys/firmware/zynqmp/health_status
+
+		Unset healty bit
+		# echo 0 > /sys/firmware/zynqmp/health_status
+
+Users:		Xilinx
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index b23bda4..4e497a3 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -479,6 +479,7 @@ static inline int zynqmp_is_valid_ioctl(u32 ioctl_id)
 	case IOCTL_READ_GGS:
 	case IOCTL_WRITE_PGGS:
 	case IOCTL_READ_PGGS:
+	case IOCTL_SET_BOOT_HEALTH_STATUS:
 		return 1;
 	default:
 		return 0;
@@ -886,6 +887,46 @@ static struct kobj_attribute zynqmp_attr_shutdown_scope =
 						__ATTR_RW(shutdown_scope);
 
 /**
+ * health_status_store - Store health_status sysfs attribute
+ * @kobj:	Kobject structure
+ * @attr:	Kobject attribute structure
+ * @buf:	User entered health_status attribute string
+ * @count:	Buffer size
+ *
+ * User-space interface for setting the boot health status.
+ * Usage: echo <value> > /sys/firmware/zynqmp/health_status
+ *
+ * Value:
+ *	1 - Set healthy bit to 1
+ *	0 - Unset healthy bit
+ *
+ * Return:	count argument if request succeeds, the corresponding error
+ *		code otherwise
+ */
+static ssize_t health_status_store(struct kobject *kobj,
+				   struct kobj_attribute *attr,
+				   const char *buf, size_t count)
+{
+	int ret;
+	unsigned int value;
+
+	ret = kstrtouint(buf, 10, &value);
+	if (ret)
+		return ret;
+
+	ret = zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0, NULL);
+	if (ret) {
+		pr_err("unable to set healthy bit value to %u\n", value);
+		return ret;
+	}
+
+	return count;
+}
+
+static struct kobj_attribute zynqmp_attr_health_status =
+						__ATTR_WO(health_status);
+
+/**
  * config_reg_store - Write config_reg sysfs attribute
  * @kobj:	Kobject structure
  * @attr:	Kobject attribute structure
@@ -1011,6 +1052,7 @@ static struct kobj_attribute zynqmp_attr_config_reg =
 
 static struct attribute *attrs[] = {
 	&zynqmp_attr_shutdown_scope.attr,
+	&zynqmp_attr_health_status.attr,
 	&zynqmp_attr_config_reg.attr,
 	NULL,
 };
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 54061de..c6f3c50 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -107,6 +107,8 @@ enum pm_ioctl_id {
 	IOCTL_READ_GGS,
 	IOCTL_WRITE_PGGS,
 	IOCTL_READ_PGGS,
+	/* Set healthy bit value*/
+	IOCTL_SET_BOOT_HEALTH_STATUS = 17,
 };
 
 enum pm_query_id {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] firmware: xilinx: Add sysfs and IOCTL to set boot health status

[Michal Simek]

On 05. 12. 19 0:29, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Add sysfs interface to set boot health status from userspace.
> Add IOCTL ID used by this interface to communicate with firmware.
> 
> If PMUFW is compiled with CHECK_HEALTHY_BOOT, it will check the
> healthy bit on FPD WDT expiration. If healthy bit is set by a user
> application running in Linux, PMUFW will do APU only restart. If
> healthy bit is not set during FPD WDT expiration, PMUFW will do
> system restart.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>

Rajan should be the first.

> Signed-off-by: Jolly Shah <jolly.shah@xilinx.com>
> ---
>  Documentation/ABI/stable/sysfs-firmware-zynqmp | 21 +++++++++++++
>  drivers/firmware/xilinx/zynqmp.c               | 42 ++++++++++++++++++++++++++
>  include/linux/firmware/xlnx-zynqmp.h           |  2 ++
>  3 files changed, 65 insertions(+)
> 
> diff --git a/Documentation/ABI/stable/sysfs-firmware-zynqmp b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> index 13e4fd2..eeae291 100644
> --- a/Documentation/ABI/stable/sysfs-firmware-zynqmp
> +++ b/Documentation/ABI/stable/sysfs-firmware-zynqmp
> @@ -80,3 +80,24 @@ Description:
>  		# echo "subsystem" > /sys/firmware/zynqmp/shutdown_scope
>  
>  Users:		Xilinx
> +
> +What:		/sys/firmware/zynqmp/health_status
> +Date:		April 2018
> +KernelVersion:	4.14.0

ditto

> +Contact:	"Rajan Vaja" <rajanv@xilinx.com>
> +Description:
> +		This sysfs interface allows to set the health status. If PMUFW
> +		is compiled with CHECK_HEALTHY_BOOT, it will check the healthy
> +		bit on FPD WDT expiration. If healthy bit is set by a user
> +		application running in Linux, PMUFW will do APU only restart. If
> +		healthy bit is not set during FPD WDT expiration, PMUFW will do
> +		system restart.
> +
> +		Usage:
> +		Set healty bit

healthy

> +		# echo 1 > /sys/firmware/zynqmp/health_status
> +
> +		Unset healty bit

ditto.

> +		# echo 0 > /sys/firmware/zynqmp/health_status
> +
> +Users:		Xilinx
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index b23bda4..4e497a3 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -479,6 +479,7 @@ static inline int zynqmp_is_valid_ioctl(u32 ioctl_id)
>  	case IOCTL_READ_GGS:
>  	case IOCTL_WRITE_PGGS:
>  	case IOCTL_READ_PGGS:
> +	case IOCTL_SET_BOOT_HEALTH_STATUS:
>  		return 1;
>  	default:
>  		return 0;
> @@ -886,6 +887,46 @@ static struct kobj_attribute zynqmp_attr_shutdown_scope =
>  						__ATTR_RW(shutdown_scope);
>  
>  /**
> + * health_status_store - Store health_status sysfs attribute
> + * @kobj:	Kobject structure
> + * @attr:	Kobject attribute structure
> + * @buf:	User entered health_status attribute string
> + * @count:	Buffer size
> + *
> + * User-space interface for setting the boot health status.
> + * Usage: echo <value> > /sys/firmware/zynqmp/health_status
> + *
> + * Value:
> + *	1 - Set healthy bit to 1
> + *	0 - Unset healthy bit
> + *
> + * Return:	count argument if request succeeds, the corresponding error
> + *		code otherwise
> + */
> +static ssize_t health_status_store(struct kobject *kobj,
> +				   struct kobj_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	int ret;
> +	unsigned int value;
> +
> +	ret = kstrtouint(buf, 10, &value);
> +	if (ret)
> +		return ret;
> +
> +	ret = zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0, NULL);
> +	if (ret) {
> +		pr_err("unable to set healthy bit value to %u\n", value);
> +		return ret;
> +	}
> +
> +	return count;
> +}
> +
> +static struct kobj_attribute zynqmp_attr_health_status =
> +						__ATTR_WO(health_status);

ditto.

> +
> +/**
>   * config_reg_store - Write config_reg sysfs attribute
>   * @kobj:	Kobject structure
>   * @attr:	Kobject attribute structure
> @@ -1011,6 +1052,7 @@ static struct kobj_attribute zynqmp_attr_config_reg =
>  
>  static struct attribute *attrs[] = {
>  	&zynqmp_attr_shutdown_scope.attr,
> +	&zynqmp_attr_health_status.attr,
>  	&zynqmp_attr_config_reg.attr,
>  	NULL,
>  };
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index 54061de..c6f3c50 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -107,6 +107,8 @@ enum pm_ioctl_id {
>  	IOCTL_READ_GGS,
>  	IOCTL_WRITE_PGGS,
>  	IOCTL_READ_PGGS,
> +	/* Set healthy bit value*/

<space>*/

> +	IOCTL_SET_BOOT_HEALTH_STATUS = 17,
>  };
>  
>  enum pm_query_id {
> 


M

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface

[Sudeep Holla]

On Wed, Dec 04, 2019 at 03:29:14PM -0800, Jolly Shah wrote:
> This patch series adds xilinx specific sysfs interface for below
> purposes:
> - Register access
> - Set shutdown scope
> - Set boot health status bit

This series defeats the whole abstraction EEMI provides. By providing
direct register accesses, you are allowing user-space to do whatever it
wants. I had NACKed this idea before. Has anything changed ?

If you need it for testing firmware, better put them in debugfs which is
off on production builds.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface

[Jolly Shah]

Hi Sudeep,

Thanks for the review.

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: Wednesday, December 18, 2019 6:46 AM
> 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>
> Subject: Re: [PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface
> 
> On Wed, Dec 04, 2019 at 03:29:14PM -0800, Jolly Shah wrote:
> > This patch series adds xilinx specific sysfs interface for below
> > purposes:
> > - Register access
> > - Set shutdown scope
> > - Set boot health status bit
> 
> This series defeats the whole abstraction EEMI provides. By providing
> direct register accesses, you are allowing user-space to do whatever it
> wants. I had NACKed this idea before. Has anything changed ?
>

Firmware checks for allowed accesses only and rejects rest. 
 
> If you need it for testing firmware, better put them in debugfs which is
> off on production builds.

Sure. Will reanalyze use cases and move to debugfs only if that suffices.

Thanks,
Jolly Shah


> 
> --
> Regards,
> Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface

[Sudeep Holla]

On Thu, Jan 02, 2020 at 09:01:58PM +0000, Jolly Shah wrote:
> Hi Sudeep,
>
> Thanks for the review.
>
> > -----Original Message-----
> > From: Sudeep Holla <sudeep.holla@arm.com>
> > Sent: Wednesday, December 18, 2019 6:46 AM
> > 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>
> > Subject: Re: [PATCH 0/5] firmware: xilinx: Add xilinx specific sysfs interface
> >
> > On Wed, Dec 04, 2019 at 03:29:14PM -0800, Jolly Shah wrote:
> > > This patch series adds xilinx specific sysfs interface for below
> > > purposes:
> > > - Register access
> > > - Set shutdown scope
> > > - Set boot health status bit
> >
> > This series defeats the whole abstraction EEMI provides. By providing
> > direct register accesses, you are allowing user-space to do whatever it
> > wants. I had NACKed this idea before. Has anything changed ?
> >
>
> Firmware checks for allowed accesses only and rejects rest.
>

If that is always the case, why not abstract them and remove this direct
register access completely. It must go or we must remove EEMI abstraction
and just provide direct register access to the entire space. I really
don't like this mix-n-match approach here.

> > If you need it for testing firmware, better put them in debugfs which is
> > off on production builds.
>
> Sure. Will reanalyze use cases and move to debugfs only if that suffices.
>

Thanks.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel