RE: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-24 Thread Jolly Shah
Thanks for review Greg,

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, January 23, 2018 12:38 AM
> To: Jolly Shah <jol...@xilinx.com>
> Cc: ard.biesheu...@linaro.org; mi...@kernel.org; m...@codeblueprint.co.uk;
> sudeep.ho...@arm.com; hkallwe...@gmail.com; keesc...@chromium.org;
> dmitry.torok...@gmail.com; michal.si...@xilinx.com; robh...@kernel.org;
> mark.rutl...@arm.com; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; Rajan Vaja
> <raj...@xilinx.com>; Jolly Shah <jol...@xilinx.com>
> Subject: Re: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface
> 
> On Wed, Jan 17, 2018 at 12:20:33PM -0800, Jolly Shah wrote:
> > Add Firmware-ggs sysfs interface which provides read/write interface
> > to global storage registers.
> >
> > Signed-off-by: Jolly Shah <jol...@xilinx.com>
> > Signed-off-by: Rajan Vaja <raj...@xilinx.com>
> > ---
> >  .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
> >  drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
> >  drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298
> +
> >  drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
> >  include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
> >  5 files changed, 360 insertions(+), 1 deletion(-)  create mode 100644
> > Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> >  create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> >
> > +
> > +#include 
> 
> That's crazy deep nesting, why?

It needs to be in include/linux to be used by other drivers and as it is Xilinx 
specific, we have it in subdirectory.

> 
> > +
> > +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 = get_eemi_ops();
> > +
> > +   if (!eemi_ops || !eemi_ops->ioctl)
> > +   return 0;
> 
> Not an error?

Fixed in v3 patch series.

> 
> > +
> > +   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
> > +   if (ret)
> > +   return ret;
> > +
> > +   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);
> 
> Minor nit, you never need to use snprintf() for a sysfs file, as you "know" 
> the size
> and you can't overflow it with just a single value.
> 
> Yeah, some tool-checkers hate to see a "raw" sprintf() call, but really, 
> ignore
> them here :)

Changed to sprint in v3 patch series.

> > +
> > +   /* Read the write value */
> > +   tok = strsep(, " ");
> > +   if (!tok) {
> > +   ret = -EFAULT;
> > +   goto err;
> > +   }
> > +
> > +   ret = kstrtol(tok, 16, );
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   goto err;
> > +   }
> 
> What exactly is the format for the data to be written here?  You do not
> document it in the ABI/ file above, and it looks to be non-trivial to 
> understand
> from the code :(
> 

Updated documentation in v3 patch series.

> > +
> > +#define CREATE_PGGS_DEVICE(dev, N) \
> > +do { \
> > +   if (device_create_file(dev, _attr_pggs##N)) \
> > +   dev_err(dev, "unable to create pggs%d attribute\n",
> > +N); \
> 
> Ick, no, just use an attribute group please.  Handles all of this mess for you
> automatically, and will unwind properly if you have an error (which this macro
> does not do.)
> 
> thanks,
> 
> greg k-h

Fixed in v3 patch series.

Thanks,
Jolly Shah



RE: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-24 Thread Jolly Shah
Thanks for review Greg,

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, January 23, 2018 12:38 AM
> To: Jolly Shah 
> Cc: ard.biesheu...@linaro.org; mi...@kernel.org; m...@codeblueprint.co.uk;
> sudeep.ho...@arm.com; hkallwe...@gmail.com; keesc...@chromium.org;
> dmitry.torok...@gmail.com; michal.si...@xilinx.com; robh...@kernel.org;
> mark.rutl...@arm.com; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; Rajan Vaja
> ; Jolly Shah 
> Subject: Re: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface
> 
> On Wed, Jan 17, 2018 at 12:20:33PM -0800, Jolly Shah wrote:
> > Add Firmware-ggs sysfs interface which provides read/write interface
> > to global storage registers.
> >
> > Signed-off-by: Jolly Shah 
> > Signed-off-by: Rajan Vaja 
> > ---
> >  .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
> >  drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
> >  drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298
> +
> >  drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
> >  include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
> >  5 files changed, 360 insertions(+), 1 deletion(-)  create mode 100644
> > Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> >  create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> >
> > +
> > +#include 
> 
> That's crazy deep nesting, why?

It needs to be in include/linux to be used by other drivers and as it is Xilinx 
specific, we have it in subdirectory.

> 
> > +
> > +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 = get_eemi_ops();
> > +
> > +   if (!eemi_ops || !eemi_ops->ioctl)
> > +   return 0;
> 
> Not an error?

Fixed in v3 patch series.

> 
> > +
> > +   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
> > +   if (ret)
> > +   return ret;
> > +
> > +   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);
> 
> Minor nit, you never need to use snprintf() for a sysfs file, as you "know" 
> the size
> and you can't overflow it with just a single value.
> 
> Yeah, some tool-checkers hate to see a "raw" sprintf() call, but really, 
> ignore
> them here :)

Changed to sprint in v3 patch series.

> > +
> > +   /* Read the write value */
> > +   tok = strsep(, " ");
> > +   if (!tok) {
> > +   ret = -EFAULT;
> > +   goto err;
> > +   }
> > +
> > +   ret = kstrtol(tok, 16, );
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   goto err;
> > +   }
> 
> What exactly is the format for the data to be written here?  You do not
> document it in the ABI/ file above, and it looks to be non-trivial to 
> understand
> from the code :(
> 

Updated documentation in v3 patch series.

> > +
> > +#define CREATE_PGGS_DEVICE(dev, N) \
> > +do { \
> > +   if (device_create_file(dev, _attr_pggs##N)) \
> > +   dev_err(dev, "unable to create pggs%d attribute\n",
> > +N); \
> 
> Ick, no, just use an attribute group please.  Handles all of this mess for you
> automatically, and will unwind properly if you have an error (which this macro
> does not do.)
> 
> thanks,
> 
> greg k-h

Fixed in v3 patch series.

Thanks,
Jolly Shah



Re: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-23 Thread Greg KH
On Wed, Jan 17, 2018 at 12:20:33PM -0800, Jolly Shah wrote:
> Add Firmware-ggs sysfs interface which provides read/write
> interface to global storage registers.
> 
> Signed-off-by: Jolly Shah 
> Signed-off-by: Rajan Vaja 
> ---
>  .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
>  drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
>  drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298 
> +
>  drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
>  include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
>  5 files changed, 360 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
>  create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> 
> diff --git a/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware 
> b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> new file mode 100644
> index 000..2483215
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> @@ -0,0 +1,33 @@
> +What:  /sys/devices/platform/zynqmp-firmware/ggs*
> +Date:  January 2018
> +KernelVersion: 4.15.0
> +Contact:   "Jolly Shah" 
> +Description:
> +   Shows 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}.
> +
> +Users: Xilinx
> +
> +What:  /sys/devices/platform/zynqmp-firmware/pggs*
> +Date:  January 2018
> +KernelVersion: 4.15.0
> +Contact:   "Jolly Shah" 
> +Description:
> +   Shows 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.
> +Users: Xilinx
> diff --git a/drivers/firmware/xilinx/zynqmp/Makefile 
> b/drivers/firmware/xilinx/zynqmp/Makefile
> index c3ec669..6629781 100644
> --- a/drivers/firmware/xilinx/zynqmp/Makefile
> +++ b/drivers/firmware/xilinx/zynqmp/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  # Makefile for Xilinx firmwares
> 
> -obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o
> +obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o firmware-ggs.o
> diff --git a/drivers/firmware/xilinx/zynqmp/firmware-ggs.c 
> b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> new file mode 100644
> index 000..be47ca2
> --- /dev/null
> +++ b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Xilinx Zynq MPSoC Firmware layer
> + *
> + *  Copyright (C) 2014-2018 Xilinx, Inc.
> + *
> + *  Jolly Shah 
> + *  Rajan Vaja 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

That's crazy deep nesting, why?

> +
> +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 = get_eemi_ops();
> +
> +   if (!eemi_ops || !eemi_ops->ioctl)
> +   return 0;

Not an error?

> +
> +   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
> +   if (ret)
> +   return ret;
> +
> +   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);

Minor nit, you never need to use snprintf() for a sysfs file, as you
"know" the size and you can't overflow it with just a single value.

Yeah, some tool-checkers hate to see a "raw" sprintf() call, but really,
ignore them here :)

> +}
> +
> +static ssize_t write_register(const char *buf, size_t count,
> + u32 ioctl_id, u32 reg)
> +{
> +   char *kern_buff;
> +   char *inbuf;
> +   char *tok;
> +   long mask;
> +   long value;
> +   int ret;
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
> +
> +   if (!eemi_ops || !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) 

Re: [PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-23 Thread Greg KH
On Wed, Jan 17, 2018 at 12:20:33PM -0800, Jolly Shah wrote:
> Add Firmware-ggs sysfs interface which provides read/write
> interface to global storage registers.
> 
> Signed-off-by: Jolly Shah 
> Signed-off-by: Rajan Vaja 
> ---
>  .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
>  drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
>  drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298 
> +
>  drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
>  include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
>  5 files changed, 360 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
>  create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> 
> diff --git a/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware 
> b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> new file mode 100644
> index 000..2483215
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
> @@ -0,0 +1,33 @@
> +What:  /sys/devices/platform/zynqmp-firmware/ggs*
> +Date:  January 2018
> +KernelVersion: 4.15.0
> +Contact:   "Jolly Shah" 
> +Description:
> +   Shows 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}.
> +
> +Users: Xilinx
> +
> +What:  /sys/devices/platform/zynqmp-firmware/pggs*
> +Date:  January 2018
> +KernelVersion: 4.15.0
> +Contact:   "Jolly Shah" 
> +Description:
> +   Shows 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.
> +Users: Xilinx
> diff --git a/drivers/firmware/xilinx/zynqmp/Makefile 
> b/drivers/firmware/xilinx/zynqmp/Makefile
> index c3ec669..6629781 100644
> --- a/drivers/firmware/xilinx/zynqmp/Makefile
> +++ b/drivers/firmware/xilinx/zynqmp/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  # Makefile for Xilinx firmwares
> 
> -obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o
> +obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o firmware-ggs.o
> diff --git a/drivers/firmware/xilinx/zynqmp/firmware-ggs.c 
> b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> new file mode 100644
> index 000..be47ca2
> --- /dev/null
> +++ b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Xilinx Zynq MPSoC Firmware layer
> + *
> + *  Copyright (C) 2014-2018 Xilinx, Inc.
> + *
> + *  Jolly Shah 
> + *  Rajan Vaja 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

That's crazy deep nesting, why?

> +
> +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 = get_eemi_ops();
> +
> +   if (!eemi_ops || !eemi_ops->ioctl)
> +   return 0;

Not an error?

> +
> +   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
> +   if (ret)
> +   return ret;
> +
> +   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);

Minor nit, you never need to use snprintf() for a sysfs file, as you
"know" the size and you can't overflow it with just a single value.

Yeah, some tool-checkers hate to see a "raw" sprintf() call, but really,
ignore them here :)

> +}
> +
> +static ssize_t write_register(const char *buf, size_t count,
> + u32 ioctl_id, u32 reg)
> +{
> +   char *kern_buff;
> +   char *inbuf;
> +   char *tok;
> +   long mask;
> +   long value;
> +   int ret;
> +   u32 ret_payload[PAYLOAD_ARG_CNT];
> +   const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
> +
> +   if (!eemi_ops || !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;
> +
> 

[PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-17 Thread Jolly Shah
Add Firmware-ggs sysfs interface which provides read/write
interface to global storage registers.

Signed-off-by: Jolly Shah 
Signed-off-by: Rajan Vaja 
---
 .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
 drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
 drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298 +
 drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
 include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
 5 files changed, 360 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
 create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c

diff --git a/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware 
b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
new file mode 100644
index 000..2483215
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
@@ -0,0 +1,33 @@
+What:  /sys/devices/platform/zynqmp-firmware/ggs*
+Date:  January 2018
+KernelVersion: 4.15.0
+Contact:   "Jolly Shah" 
+Description:
+   Shows 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}.
+
+Users: Xilinx
+
+What:  /sys/devices/platform/zynqmp-firmware/pggs*
+Date:  January 2018
+KernelVersion: 4.15.0
+Contact:   "Jolly Shah" 
+Description:
+   Shows 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.
+Users: Xilinx
diff --git a/drivers/firmware/xilinx/zynqmp/Makefile 
b/drivers/firmware/xilinx/zynqmp/Makefile
index c3ec669..6629781 100644
--- a/drivers/firmware/xilinx/zynqmp/Makefile
+++ b/drivers/firmware/xilinx/zynqmp/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0+
 # Makefile for Xilinx firmwares

-obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o
+obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o firmware-ggs.o
diff --git a/drivers/firmware/xilinx/zynqmp/firmware-ggs.c 
b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
new file mode 100644
index 000..be47ca2
--- /dev/null
+++ b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Xilinx Zynq MPSoC Firmware layer
+ *
+ *  Copyright (C) 2014-2018 Xilinx, Inc.
+ *
+ *  Jolly Shah 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+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 = get_eemi_ops();
+
+   if (!eemi_ops || !eemi_ops->ioctl)
+   return 0;
+
+   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
+   if (ret)
+   return ret;
+
+   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);
+}
+
+static ssize_t write_register(const char *buf, size_t count,
+ u32 ioctl_id, u32 reg)
+{
+   char *kern_buff;
+   char *inbuf;
+   char *tok;
+   long mask;
+   long value;
+   int ret;
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
+
+   if (!eemi_ops || !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(, " ");
+   if (!tok) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   ret = kstrtol(tok, 16, );
+   if (ret) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   /* Read the write value */
+   tok = strsep(, " ");
+   if (!tok) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   ret = kstrtol(tok, 16, );
+   if (ret) {
+   ret = 

[PATCH v2 3/4] drivers: firmware: xilinx: Add sysfs interface

2018-01-17 Thread Jolly Shah
Add Firmware-ggs sysfs interface which provides read/write
interface to global storage registers.

Signed-off-by: Jolly Shah 
Signed-off-by: Rajan Vaja 
---
 .../ABI/stable/sysfs-driver-zynqmp-firmware|  33 +++
 drivers/firmware/xilinx/zynqmp/Makefile|   2 +-
 drivers/firmware/xilinx/zynqmp/firmware-ggs.c  | 298 +
 drivers/firmware/xilinx/zynqmp/firmware.c  |  26 ++
 include/linux/firmware/xilinx/zynqmp/firmware.h|   2 +
 5 files changed, 360 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
 create mode 100644 drivers/firmware/xilinx/zynqmp/firmware-ggs.c

diff --git a/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware 
b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
new file mode 100644
index 000..2483215
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-zynqmp-firmware
@@ -0,0 +1,33 @@
+What:  /sys/devices/platform/zynqmp-firmware/ggs*
+Date:  January 2018
+KernelVersion: 4.15.0
+Contact:   "Jolly Shah" 
+Description:
+   Shows 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}.
+
+Users: Xilinx
+
+What:  /sys/devices/platform/zynqmp-firmware/pggs*
+Date:  January 2018
+KernelVersion: 4.15.0
+Contact:   "Jolly Shah" 
+Description:
+   Shows 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.
+Users: Xilinx
diff --git a/drivers/firmware/xilinx/zynqmp/Makefile 
b/drivers/firmware/xilinx/zynqmp/Makefile
index c3ec669..6629781 100644
--- a/drivers/firmware/xilinx/zynqmp/Makefile
+++ b/drivers/firmware/xilinx/zynqmp/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0+
 # Makefile for Xilinx firmwares

-obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o
+obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware.o firmware-ggs.o
diff --git a/drivers/firmware/xilinx/zynqmp/firmware-ggs.c 
b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
new file mode 100644
index 000..be47ca2
--- /dev/null
+++ b/drivers/firmware/xilinx/zynqmp/firmware-ggs.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Xilinx Zynq MPSoC Firmware layer
+ *
+ *  Copyright (C) 2014-2018 Xilinx, Inc.
+ *
+ *  Jolly Shah 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+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 = get_eemi_ops();
+
+   if (!eemi_ops || !eemi_ops->ioctl)
+   return 0;
+
+   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
+   if (ret)
+   return ret;
+
+   return snprintf(buf, PAGE_SIZE, "0x%x\n", ret_payload[1]);
+}
+
+static ssize_t write_register(const char *buf, size_t count,
+ u32 ioctl_id, u32 reg)
+{
+   char *kern_buff;
+   char *inbuf;
+   char *tok;
+   long mask;
+   long value;
+   int ret;
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
+
+   if (!eemi_ops || !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(, " ");
+   if (!tok) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   ret = kstrtol(tok, 16, );
+   if (ret) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   /* Read the write value */
+   tok = strsep(, " ");
+   if (!tok) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   ret = kstrtol(tok, 16, );
+   if (ret) {
+   ret = -EFAULT;
+   goto err;
+   }
+
+   ret = eemi_ops->ioctl(0, ioctl_id, reg, 0, ret_payload);
+