All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] soc: fsl: add DPAA2 console support
@ 2019-03-26 19:16 Ioana Ciornei
  2019-04-03 19:55 ` Li Yang
  0 siblings, 1 reply; 5+ messages in thread
From: Ioana Ciornei @ 2019-03-26 19:16 UTC (permalink / raw)
  To: Leo Li; +Cc: linux-kernel, Ioana Ciornei, Roy Pledge

This patch adds DPAA2 MC and AIOP console log support.

The platform driver probes on the "fsl,dpaa2-console" device tree node
which describes the base firmware address needed in order to infer the
start address of both firmware logs: MC and AIOP.
It then exports two misc char devices which can be used to dump
the needed logs.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
---
 drivers/soc/fsl/Kconfig         |  10 ++
 drivers/soc/fsl/Makefile        |   1 +
 drivers/soc/fsl/dpaa2-console.c | 306 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 317 insertions(+)
 create mode 100644 drivers/soc/fsl/dpaa2-console.c

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 61f8e1433d0a..9fe8222c3968 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -29,4 +29,14 @@ config FSL_MC_DPIO
 	  other DPAA2 objects. This driver does not expose the DPIO
 	  objects individually, but groups them under a service layer
 	  API.
+
+config DPAA2_CONSOLE
+	tristate "QorIQ DPAA2 console driver"
+	depends on OF && (ARCH_LAYERSCAPE || (COMPILE_TEST && (ARM || ARM64 || X86_LOCAL_APIC || PPC)))
+	default y
+	help
+	  Console driver for DPAA2 platforms. Exports 2 char devices,
+	  /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
+	  which can be used to dump the Management Complex and AIOP
+	  firmware logs.
 endmenu
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 803ef1bfb5ff..57762c9fc7da 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)		+= qe/
 obj-$(CONFIG_CPM)			+= qe/
 obj-$(CONFIG_FSL_GUTS)			+= guts.o
 obj-$(CONFIG_FSL_MC_DPIO) 		+= dpio/
+obj-$(CONFIG_DPAA2_CONSOLE)		+= dpaa2-console.o
diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
new file mode 100644
index 000000000000..21a5e121f87b
--- /dev/null
+++ b/drivers/soc/fsl/dpaa2-console.c
@@ -0,0 +1,306 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2015-2016 Freescale Semiconductor Inc.
+ * Copyright 2018 NXP
+ */
+
+#define pr_fmt(fmt) "dpaa2-console: " fmt
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/io.h>
+
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("Roy Pledge <roy.pledge@nxp.com>");
+MODULE_DESCRIPTION("DPAA2 console driver");
+
+/* MC firmware base low/high registers indexes */
+#define MCFBALR_OFFSET 0
+#define MCFBAHR_OFFSET 1
+
+/* Bit masks used to get the most/least significant part of the MC base addr */
+#define MC_FW_ADDR_MASK_HIGH 0x1FFFF
+#define MC_FW_ADDR_MASK_LOW  0xE0000000
+
+#define MC_BUFFER_OFFSET 0x01000000
+#define MC_BUFFER_SIZE   (1024 * 1024 * 16)
+#define MC_OFFSET_DELTA  MC_BUFFER_OFFSET
+
+#define AIOP_BUFFER_OFFSET 0x06000000
+#define AIOP_BUFFER_SIZE   (1024 * 1024 * 16)
+#define AIOP_OFFSET_DELTA  0
+
+#define LOG_HEADER_FLAG_BUFFER_WRAPAROUND 0x80000000
+#define LAST_BYTE(a) ((a) & ~(LOG_HEADER_FLAG_BUFFER_WRAPAROUND))
+
+/* MC and AIOP Magic words */
+#define MAGIC_MC   0x4d430100
+#define MAGIC_AIOP 0x41494F50
+
+struct log_header {
+	__le32 magic_word;
+	char reserved[4];
+	__le32 buf_start;
+	__le32 buf_length;
+	__le32 last_byte;
+};
+
+struct console_data {
+	char *map_addr;
+	struct log_header *hdr;
+	char *start_addr;
+	char *end_addr;
+	char *end_of_data;
+	char *cur_ptr;
+};
+
+struct resource mc_base_addr;
+
+static inline void adjust_end(struct console_data *cd)
+{
+	u32 last_byte = readl(&cd->hdr->last_byte);
+
+	cd->end_of_data = cd->start_addr + LAST_BYTE(last_byte);
+}
+
+static u64 get_mc_fw_base_address(void)
+{
+	u64 mcfwbase = 0ULL;
+	u32 *mcfbaregs;
+
+	mcfbaregs = (u32 *)ioremap(mc_base_addr.start,
+				   resource_size(&mc_base_addr));
+	if (!mcfbaregs) {
+		pr_err("could not map MC Firmaware Base registers\n");
+		return -EIO;
+	}
+
+	mcfwbase  = readl(mcfbaregs + MCFBAHR_OFFSET) & MC_FW_ADDR_MASK_HIGH;
+	mcfwbase <<= 32;
+	mcfwbase |= readl(mcfbaregs + MCFBALR_OFFSET) & MC_FW_ADDR_MASK_LOW;
+	iounmap(mcfbaregs);
+
+	pr_debug("MC base address at 0x%016llx\n", mcfwbase);
+	return mcfwbase;
+}
+
+static ssize_t dpaa2_console_size(struct console_data *cd)
+{
+	ssize_t size;
+
+	if (cd->cur_ptr <= cd->end_of_data)
+		size = cd->end_of_data - cd->cur_ptr;
+	else
+		size = (cd->end_addr - cd->cur_ptr) +
+			(cd->end_of_data - cd->start_addr);
+
+	return size;
+}
+
+static int dpaa2_generic_console_open(struct inode *node, struct file *fp,
+				      u64 offset, u64 size,
+				      u32 expected_magic,
+				      u32 offset_delta)
+{
+	u32 read_magic, wrapped, last_byte, buf_start, buf_length;
+	struct console_data *cd;
+	u64 base_addr;
+	int err;
+
+	cd = kmalloc(sizeof(*cd), GFP_KERNEL);
+	if (!cd)
+		return -ENOMEM;
+
+	base_addr = get_mc_fw_base_address();
+	if (base_addr < 0)
+		return -EIO;
+
+	cd->map_addr = ioremap(base_addr + offset, size);
+	if (!cd->map_addr) {
+		pr_err("cannot map console log memory\n");
+		err = -EIO;
+		goto err_ioremap;
+	}
+
+	cd->hdr = (struct log_header *)cd->map_addr;
+	read_magic = readl(&cd->hdr->magic_word);
+	last_byte = readl(&cd->hdr->last_byte);
+	buf_start = readl(&cd->hdr->buf_start);
+	buf_length = readl(&cd->hdr->buf_length);
+
+	if (read_magic != expected_magic) {
+		pr_warn("expected = %08x, read = %08x\n",
+			expected_magic, read_magic);
+		err = -EIO;
+		goto err_magic;
+	}
+
+	cd->start_addr = cd->map_addr + buf_start - offset_delta;
+	cd->end_addr = cd->start_addr + buf_length;
+
+	wrapped = last_byte & LOG_HEADER_FLAG_BUFFER_WRAPAROUND;
+
+	adjust_end(cd);
+	if (wrapped && cd->end_of_data != cd->end_addr)
+		cd->cur_ptr = cd->end_of_data + 1;
+	else
+		cd->cur_ptr = cd->start_addr;
+
+	fp->private_data = cd;
+
+	return 0;
+
+err_magic:
+	iounmap(cd->map_addr);
+
+err_ioremap:
+	kfree(cd);
+
+	return err;
+}
+
+static int dpaa2_mc_console_open(struct inode *node, struct file *fp)
+{
+	return dpaa2_generic_console_open(node, fp,
+					  MC_BUFFER_OFFSET, MC_BUFFER_SIZE,
+					  MAGIC_MC, MC_OFFSET_DELTA);
+}
+
+static int dpaa2_aiop_console_open(struct inode *node, struct file *fp)
+{
+	return dpaa2_generic_console_open(node, fp,
+					  AIOP_BUFFER_OFFSET, AIOP_BUFFER_SIZE,
+					  MAGIC_AIOP, AIOP_OFFSET_DELTA);
+}
+
+static int dpaa2_console_close(struct inode *node, struct file *fp)
+{
+	struct console_data *cd = fp->private_data;
+
+	iounmap(cd->map_addr);
+	kfree(cd);
+	return 0;
+}
+
+static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
+				  size_t count, loff_t *f_pos)
+{
+	struct console_data *cd = fp->private_data;
+	size_t bytes = dpaa2_console_size(cd);
+	size_t bytes_end = cd->end_addr - cd->cur_ptr;
+	size_t written = 0;
+
+	/* Check if we need to adjust the end of data addr */
+	adjust_end(cd);
+
+	if (cd->end_of_data == cd->cur_ptr)
+		return 0;
+
+	if (count < bytes)
+		bytes = count;
+
+	if (bytes > bytes_end) {
+		if (copy_to_user(buf, cd->cur_ptr, bytes_end))
+			return -EFAULT;
+		buf += bytes_end;
+		cd->cur_ptr = cd->start_addr;
+		bytes -= bytes_end;
+		written += bytes_end;
+	}
+
+	if (copy_to_user(buf, cd->cur_ptr, bytes))
+		return -EFAULT;
+	cd->cur_ptr += bytes;
+	written += bytes;
+
+	return written;
+}
+
+static const struct file_operations dpaa2_mc_console_fops = {
+	.owner          = THIS_MODULE,
+	.open           = dpaa2_mc_console_open,
+	.release        = dpaa2_console_close,
+	.read           = dpaa2_console_read,
+};
+
+static struct miscdevice dpaa2_mc_console_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "dpaa2_mc_console",
+	.fops = &dpaa2_mc_console_fops
+};
+
+static const struct file_operations dpaa2_aiop_console_fops = {
+	.owner          = THIS_MODULE,
+	.open           = dpaa2_aiop_console_open,
+	.release        = dpaa2_console_close,
+	.read           = dpaa2_console_read,
+};
+
+static struct miscdevice dpaa2_aiop_console_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "dpaa2_aiop_console",
+	.fops = &dpaa2_aiop_console_fops
+};
+
+static int dpaa2_console_probe(struct platform_device *pdev)
+{
+	int error;
+
+	error = of_address_to_resource(pdev->dev.of_node, 0, &mc_base_addr);
+	if (error < 0) {
+		pr_err("of_address_to_resource() failed for %pOF with %d\n",
+		       pdev->dev.of_node, error);
+		return error;
+	}
+
+	error = misc_register(&dpaa2_mc_console_dev);
+	if (error) {
+		pr_err("dpaa2-console: cannot register device %s\n",
+		       dpaa2_mc_console_dev.name);
+		goto err_register_mc;
+	}
+
+	error = misc_register(&dpaa2_aiop_console_dev);
+	if (error) {
+		pr_err("dpaa2-console: cannot register device %s\n",
+		       dpaa2_aiop_console_dev.name);
+		goto err_register_aiop;
+	}
+
+	return 0;
+
+err_register_aiop:
+	misc_deregister(&dpaa2_mc_console_dev);
+err_register_mc:
+	return error;
+}
+
+static int dpaa2_console_remove(struct platform_device *pdev)
+{
+	misc_deregister(&dpaa2_mc_console_dev);
+	misc_deregister(&dpaa2_aiop_console_dev);
+
+	return 0;
+}
+
+static const struct of_device_id dpaa2_console_match_table[] = {
+	{ .compatible = "fsl,dpaa2-console",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, dpaa2_console_match_table);
+
+static struct platform_driver dpaa2_console_driver = {
+	.driver = {
+		   .name = "dpaa2-console",
+		   .pm = NULL,
+		   .of_match_table = dpaa2_console_match_table,
+		   },
+	.probe = dpaa2_console_probe,
+	.remove = dpaa2_console_remove,
+};
+module_platform_driver(dpaa2_console_driver);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] soc: fsl: add DPAA2 console support
  2019-03-26 19:16 [PATCH] soc: fsl: add DPAA2 console support Ioana Ciornei
@ 2019-04-03 19:55 ` Li Yang
  2019-04-04 10:48   ` Ioana Ciornei
  0 siblings, 1 reply; 5+ messages in thread
From: Li Yang @ 2019-04-03 19:55 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: linux-kernel, Roy Pledge

On Tue, Mar 26, 2019 at 2:17 PM Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
>
> This patch adds DPAA2 MC and AIOP console log support.
>
> The platform driver probes on the "fsl,dpaa2-console" device tree node
> which describes the base firmware address needed in order to infer the
> start address of both firmware logs: MC and AIOP.
> It then exports two misc char devices which can be used to dump
> the needed logs.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
> ---
>  drivers/soc/fsl/Kconfig         |  10 ++
>  drivers/soc/fsl/Makefile        |   1 +
>  drivers/soc/fsl/dpaa2-console.c | 306 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 317 insertions(+)
>  create mode 100644 drivers/soc/fsl/dpaa2-console.c
>
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index 61f8e1433d0a..9fe8222c3968 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -29,4 +29,14 @@ config FSL_MC_DPIO
>           other DPAA2 objects. This driver does not expose the DPIO
>           objects individually, but groups them under a service layer
>           API.
> +
> +config DPAA2_CONSOLE
> +       tristate "QorIQ DPAA2 console driver"
> +       depends on OF && (ARCH_LAYERSCAPE || (COMPILE_TEST && (ARM || ARM64 || X86_LOCAL_APIC || PPC)))

I don't know why FSL_MC driver added there architectures dependency
for COMPILE_TEST, but is this driver also only buildable on these
architectures?  Can we really just remove these dependencies?

> +       default y
> +       help
> +         Console driver for DPAA2 platforms. Exports 2 char devices,
> +         /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
> +         which can be used to dump the Management Complex and AIOP
> +         firmware logs.
>  endmenu
> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> index 803ef1bfb5ff..57762c9fc7da 100644
> --- a/drivers/soc/fsl/Makefile
> +++ b/drivers/soc/fsl/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)              += qe/
>  obj-$(CONFIG_CPM)                      += qe/
>  obj-$(CONFIG_FSL_GUTS)                 += guts.o
>  obj-$(CONFIG_FSL_MC_DPIO)              += dpio/
> +obj-$(CONFIG_DPAA2_CONSOLE)            += dpaa2-console.o
> diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
> new file mode 100644
> index 000000000000..21a5e121f87b
> --- /dev/null
> +++ b/drivers/soc/fsl/dpaa2-console.c
> @@ -0,0 +1,306 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)

Probably good to have a one-line description of the driver?

> +/* Copyright 2015-2016 Freescale Semiconductor Inc.
> + * Copyright 2018 NXP
> + */
> +
> +#define pr_fmt(fmt) "dpaa2-console: " fmt
> +
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/miscdevice.h>
> +#include <linux/uaccess.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/io.h>
> +
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("Roy Pledge <roy.pledge@nxp.com>");
> +MODULE_DESCRIPTION("DPAA2 console driver");

These normally go to the bottom of a driver.

> +
> +/* MC firmware base low/high registers indexes */
> +#define MCFBALR_OFFSET 0
> +#define MCFBAHR_OFFSET 1
> +
> +/* Bit masks used to get the most/least significant part of the MC base addr */
> +#define MC_FW_ADDR_MASK_HIGH 0x1FFFF
> +#define MC_FW_ADDR_MASK_LOW  0xE0000000
> +
> +#define MC_BUFFER_OFFSET 0x01000000
> +#define MC_BUFFER_SIZE   (1024 * 1024 * 16)
> +#define MC_OFFSET_DELTA  MC_BUFFER_OFFSET
> +
> +#define AIOP_BUFFER_OFFSET 0x06000000
> +#define AIOP_BUFFER_SIZE   (1024 * 1024 * 16)
> +#define AIOP_OFFSET_DELTA  0
> +
> +#define LOG_HEADER_FLAG_BUFFER_WRAPAROUND 0x80000000
> +#define LAST_BYTE(a) ((a) & ~(LOG_HEADER_FLAG_BUFFER_WRAPAROUND))
> +
> +/* MC and AIOP Magic words */
> +#define MAGIC_MC   0x4d430100
> +#define MAGIC_AIOP 0x41494F50
> +
> +struct log_header {
> +       __le32 magic_word;
> +       char reserved[4];
> +       __le32 buf_start;
> +       __le32 buf_length;
> +       __le32 last_byte;
> +};
> +
> +struct console_data {
> +       char *map_addr;
> +       struct log_header *hdr;
> +       char *start_addr;
> +       char *end_addr;
> +       char *end_of_data;
> +       char *cur_ptr;
> +};
> +
> +struct resource mc_base_addr;
> +
> +static inline void adjust_end(struct console_data *cd)
> +{
> +       u32 last_byte = readl(&cd->hdr->last_byte);
> +
> +       cd->end_of_data = cd->start_addr + LAST_BYTE(last_byte);
> +}
> +
> +static u64 get_mc_fw_base_address(void)
> +{
> +       u64 mcfwbase = 0ULL;
> +       u32 *mcfbaregs;
> +
> +       mcfbaregs = (u32 *)ioremap(mc_base_addr.start,
> +                                  resource_size(&mc_base_addr));
> +       if (!mcfbaregs) {
> +               pr_err("could not map MC Firmaware Base registers\n");
> +               return -EIO;
> +       }
> +
> +       mcfwbase  = readl(mcfbaregs + MCFBAHR_OFFSET) & MC_FW_ADDR_MASK_HIGH;
> +       mcfwbase <<= 32;
> +       mcfwbase |= readl(mcfbaregs + MCFBALR_OFFSET) & MC_FW_ADDR_MASK_LOW;
> +       iounmap(mcfbaregs);
> +
> +       pr_debug("MC base address at 0x%016llx\n", mcfwbase);
> +       return mcfwbase;
> +}
> +
> +static ssize_t dpaa2_console_size(struct console_data *cd)
> +{
> +       ssize_t size;
> +
> +       if (cd->cur_ptr <= cd->end_of_data)
> +               size = cd->end_of_data - cd->cur_ptr;
> +       else
> +               size = (cd->end_addr - cd->cur_ptr) +
> +                       (cd->end_of_data - cd->start_addr);
> +
> +       return size;
> +}
> +
> +static int dpaa2_generic_console_open(struct inode *node, struct file *fp,
> +                                     u64 offset, u64 size,
> +                                     u32 expected_magic,
> +                                     u32 offset_delta)
> +{
> +       u32 read_magic, wrapped, last_byte, buf_start, buf_length;
> +       struct console_data *cd;
> +       u64 base_addr;
> +       int err;
> +
> +       cd = kmalloc(sizeof(*cd), GFP_KERNEL);
> +       if (!cd)
> +               return -ENOMEM;
> +
> +       base_addr = get_mc_fw_base_address();
> +       if (base_addr < 0)

We are leaking cd structure here.

> +               return -EIO;
> +
> +       cd->map_addr = ioremap(base_addr + offset, size);
> +       if (!cd->map_addr) {
> +               pr_err("cannot map console log memory\n");
> +               err = -EIO;
> +               goto err_ioremap;
> +       }
> +
> +       cd->hdr = (struct log_header *)cd->map_addr;
> +       read_magic = readl(&cd->hdr->magic_word);
> +       last_byte = readl(&cd->hdr->last_byte);
> +       buf_start = readl(&cd->hdr->buf_start);
> +       buf_length = readl(&cd->hdr->buf_length);
> +
> +       if (read_magic != expected_magic) {
> +               pr_warn("expected = %08x, read = %08x\n",
> +                       expected_magic, read_magic);
> +               err = -EIO;
> +               goto err_magic;
> +       }
> +
> +       cd->start_addr = cd->map_addr + buf_start - offset_delta;
> +       cd->end_addr = cd->start_addr + buf_length;
> +
> +       wrapped = last_byte & LOG_HEADER_FLAG_BUFFER_WRAPAROUND;
> +
> +       adjust_end(cd);
> +       if (wrapped && cd->end_of_data != cd->end_addr)
> +               cd->cur_ptr = cd->end_of_data + 1;
> +       else
> +               cd->cur_ptr = cd->start_addr;
> +
> +       fp->private_data = cd;
> +
> +       return 0;
> +
> +err_magic:
> +       iounmap(cd->map_addr);
> +
> +err_ioremap:
> +       kfree(cd);
> +
> +       return err;
> +}
> +
> +static int dpaa2_mc_console_open(struct inode *node, struct file *fp)
> +{
> +       return dpaa2_generic_console_open(node, fp,
> +                                         MC_BUFFER_OFFSET, MC_BUFFER_SIZE,
> +                                         MAGIC_MC, MC_OFFSET_DELTA);
> +}
> +
> +static int dpaa2_aiop_console_open(struct inode *node, struct file *fp)
> +{
> +       return dpaa2_generic_console_open(node, fp,
> +                                         AIOP_BUFFER_OFFSET, AIOP_BUFFER_SIZE,
> +                                         MAGIC_AIOP, AIOP_OFFSET_DELTA);
> +}
> +
> +static int dpaa2_console_close(struct inode *node, struct file *fp)
> +{
> +       struct console_data *cd = fp->private_data;
> +
> +       iounmap(cd->map_addr);
> +       kfree(cd);
> +       return 0;
> +}
> +
> +static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
> +                                 size_t count, loff_t *f_pos)
> +{
> +       struct console_data *cd = fp->private_data;
> +       size_t bytes = dpaa2_console_size(cd);
> +       size_t bytes_end = cd->end_addr - cd->cur_ptr;
> +       size_t written = 0;
> +
> +       /* Check if we need to adjust the end of data addr */
> +       adjust_end(cd);
> +
> +       if (cd->end_of_data == cd->cur_ptr)
> +               return 0;
> +
> +       if (count < bytes)
> +               bytes = count;
> +
> +       if (bytes > bytes_end) {
> +               if (copy_to_user(buf, cd->cur_ptr, bytes_end))
> +                       return -EFAULT;
> +               buf += bytes_end;
> +               cd->cur_ptr = cd->start_addr;
> +               bytes -= bytes_end;
> +               written += bytes_end;
> +       }
> +
> +       if (copy_to_user(buf, cd->cur_ptr, bytes))
> +               return -EFAULT;
> +       cd->cur_ptr += bytes;
> +       written += bytes;
> +
> +       return written;
> +}
> +
> +static const struct file_operations dpaa2_mc_console_fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = dpaa2_mc_console_open,
> +       .release        = dpaa2_console_close,
> +       .read           = dpaa2_console_read,
> +};
> +
> +static struct miscdevice dpaa2_mc_console_dev = {
> +       .minor = MISC_DYNAMIC_MINOR,
> +       .name = "dpaa2_mc_console",
> +       .fops = &dpaa2_mc_console_fops
> +};
> +
> +static const struct file_operations dpaa2_aiop_console_fops = {
> +       .owner          = THIS_MODULE,
> +       .open           = dpaa2_aiop_console_open,
> +       .release        = dpaa2_console_close,
> +       .read           = dpaa2_console_read,
> +};
> +
> +static struct miscdevice dpaa2_aiop_console_dev = {
> +       .minor = MISC_DYNAMIC_MINOR,
> +       .name = "dpaa2_aiop_console",
> +       .fops = &dpaa2_aiop_console_fops
> +};
> +
> +static int dpaa2_console_probe(struct platform_device *pdev)
> +{
> +       int error;
> +
> +       error = of_address_to_resource(pdev->dev.of_node, 0, &mc_base_addr);
> +       if (error < 0) {
> +               pr_err("of_address_to_resource() failed for %pOF with %d\n",
> +                      pdev->dev.of_node, error);
> +               return error;
> +       }
> +
> +       error = misc_register(&dpaa2_mc_console_dev);
> +       if (error) {
> +               pr_err("dpaa2-console: cannot register device %s\n",
> +                      dpaa2_mc_console_dev.name);

The pr_fmt already have the dpaa2-console prefix.

> +               goto err_register_mc;
> +       }
> +
> +       error = misc_register(&dpaa2_aiop_console_dev);
> +       if (error) {
> +               pr_err("dpaa2-console: cannot register device %s\n",
> +                      dpaa2_aiop_console_dev.name);

The pr_fmt already have the dpaa2-console prefix.

> +               goto err_register_aiop;
> +       }
> +
> +       return 0;
> +
> +err_register_aiop:
> +       misc_deregister(&dpaa2_mc_console_dev);
> +err_register_mc:
> +       return error;
> +}
> +
> +static int dpaa2_console_remove(struct platform_device *pdev)
> +{
> +       misc_deregister(&dpaa2_mc_console_dev);
> +       misc_deregister(&dpaa2_aiop_console_dev);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id dpaa2_console_match_table[] = {
> +       { .compatible = "fsl,dpaa2-console",},
> +       {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, dpaa2_console_match_table);
> +
> +static struct platform_driver dpaa2_console_driver = {
> +       .driver = {
> +                  .name = "dpaa2-console",
> +                  .pm = NULL,
> +                  .of_match_table = dpaa2_console_match_table,
> +                  },
> +       .probe = dpaa2_console_probe,
> +       .remove = dpaa2_console_remove,
> +};
> +module_platform_driver(dpaa2_console_driver);
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] soc: fsl: add DPAA2 console support
  2019-04-03 19:55 ` Li Yang
@ 2019-04-04 10:48   ` Ioana Ciornei
  2019-04-04 18:20     ` Leo Li
  0 siblings, 1 reply; 5+ messages in thread
From: Ioana Ciornei @ 2019-04-04 10:48 UTC (permalink / raw)
  To: Leo Li; +Cc: linux-kernel, Roy Pledge

On 4/3/19 10:55 PM, Li Yang wrote:
> On Tue, Mar 26, 2019 at 2:17 PM Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
>>
>> This patch adds DPAA2 MC and AIOP console log support.
>>
>> The platform driver probes on the "fsl,dpaa2-console" device tree node
>> which describes the base firmware address needed in order to infer the
>> start address of both firmware logs: MC and AIOP.
>> It then exports two misc char devices which can be used to dump
>> the needed logs.
>>
>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>> Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
>> ---
>>   drivers/soc/fsl/Kconfig         |  10 ++
>>   drivers/soc/fsl/Makefile        |   1 +
>>   drivers/soc/fsl/dpaa2-console.c | 306 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 317 insertions(+)
>>   create mode 100644 drivers/soc/fsl/dpaa2-console.c
>>
>> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
>> index 61f8e1433d0a..9fe8222c3968 100644
>> --- a/drivers/soc/fsl/Kconfig
>> +++ b/drivers/soc/fsl/Kconfig
>> @@ -29,4 +29,14 @@ config FSL_MC_DPIO
>>            other DPAA2 objects. This driver does not expose the DPIO
>>            objects individually, but groups them under a service layer
>>            API.
>> +
>> +config DPAA2_CONSOLE
>> +       tristate "QorIQ DPAA2 console driver"
>> +       depends on OF && (ARCH_LAYERSCAPE || (COMPILE_TEST && (ARM || ARM64 || X86_LOCAL_APIC || PPC)))
> 
> I don't know why FSL_MC driver added there architectures dependency
> for COMPILE_TEST, but is this driver also only buildable on these
> architectures?  Can we really just remove these dependencies?
> 

The initial reasoning behind the dependency on the COMPILE_TEST config
was to catch early build problems on any build configuration.
And yes, the dpaa2-console can be compiled without a problem on these
architectures.

In this case, I would suggest to leave the dependencies as they are but 
if you want I can also remove them.

>> +       default y
>> +       help
>> +         Console driver for DPAA2 platforms. Exports 2 char devices,
>> +         /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
>> +         which can be used to dump the Management Complex and AIOP
>> +         firmware logs.
>>   endmenu
>> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
>> index 803ef1bfb5ff..57762c9fc7da 100644
>> --- a/drivers/soc/fsl/Makefile
>> +++ b/drivers/soc/fsl/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)              += qe/
>>   obj-$(CONFIG_CPM)                      += qe/
>>   obj-$(CONFIG_FSL_GUTS)                 += guts.o
>>   obj-$(CONFIG_FSL_MC_DPIO)              += dpio/
>> +obj-$(CONFIG_DPAA2_CONSOLE)            += dpaa2-console.o
>> diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
>> new file mode 100644
>> index 000000000000..21a5e121f87b
>> --- /dev/null
>> +++ b/drivers/soc/fsl/dpaa2-console.c
>> @@ -0,0 +1,306 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> 
> Probably good to have a one-line description of the driver?

Will add one in the next version.

> 
>> +/* Copyright 2015-2016 Freescale Semiconductor Inc.
>> + * Copyright 2018 NXP
>> + */
>> +
>> +#define pr_fmt(fmt) "dpaa2-console: " fmt
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_address.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/slab.h>
>> +#include <linux/fs.h>
>> +#include <linux/io.h>
>> +
>> +MODULE_LICENSE("Dual BSD/GPL");
>> +MODULE_AUTHOR("Roy Pledge <roy.pledge@nxp.com>");
>> +MODULE_DESCRIPTION("DPAA2 console driver");
> 
> These normally go to the bottom of a driver.

Will move them.

> 
>> +
>> +/* MC firmware base low/high registers indexes */
>> +#define MCFBALR_OFFSET 0
>> +#define MCFBAHR_OFFSET 1
>> +
>> +/* Bit masks used to get the most/least significant part of the MC base addr */
>> +#define MC_FW_ADDR_MASK_HIGH 0x1FFFF
>> +#define MC_FW_ADDR_MASK_LOW  0xE0000000
>> +
>> +#define MC_BUFFER_OFFSET 0x01000000
>> +#define MC_BUFFER_SIZE   (1024 * 1024 * 16)
>> +#define MC_OFFSET_DELTA  MC_BUFFER_OFFSET
>> +
>> +#define AIOP_BUFFER_OFFSET 0x06000000
>> +#define AIOP_BUFFER_SIZE   (1024 * 1024 * 16)
>> +#define AIOP_OFFSET_DELTA  0
>> +
>> +#define LOG_HEADER_FLAG_BUFFER_WRAPAROUND 0x80000000
>> +#define LAST_BYTE(a) ((a) & ~(LOG_HEADER_FLAG_BUFFER_WRAPAROUND))
>> +
>> +/* MC and AIOP Magic words */
>> +#define MAGIC_MC   0x4d430100
>> +#define MAGIC_AIOP 0x41494F50
>> +
>> +struct log_header {
>> +       __le32 magic_word;
>> +       char reserved[4];
>> +       __le32 buf_start;
>> +       __le32 buf_length;
>> +       __le32 last_byte;
>> +};
>> +
>> +struct console_data {
>> +       char *map_addr;
>> +       struct log_header *hdr;
>> +       char *start_addr;
>> +       char *end_addr;
>> +       char *end_of_data;
>> +       char *cur_ptr;
>> +};
>> +
>> +struct resource mc_base_addr;
>> +
>> +static inline void adjust_end(struct console_data *cd)
>> +{
>> +       u32 last_byte = readl(&cd->hdr->last_byte);
>> +
>> +       cd->end_of_data = cd->start_addr + LAST_BYTE(last_byte);
>> +}
>> +
>> +static u64 get_mc_fw_base_address(void)
>> +{
>> +       u64 mcfwbase = 0ULL;
>> +       u32 *mcfbaregs;
>> +
>> +       mcfbaregs = (u32 *)ioremap(mc_base_addr.start,
>> +                                  resource_size(&mc_base_addr));
>> +       if (!mcfbaregs) {
>> +               pr_err("could not map MC Firmaware Base registers\n");
>> +               return -EIO;
>> +       }
>> +
>> +       mcfwbase  = readl(mcfbaregs + MCFBAHR_OFFSET) & MC_FW_ADDR_MASK_HIGH;
>> +       mcfwbase <<= 32;
>> +       mcfwbase |= readl(mcfbaregs + MCFBALR_OFFSET) & MC_FW_ADDR_MASK_LOW;
>> +       iounmap(mcfbaregs);
>> +
>> +       pr_debug("MC base address at 0x%016llx\n", mcfwbase);
>> +       return mcfwbase;
>> +}
>> +
>> +static ssize_t dpaa2_console_size(struct console_data *cd)
>> +{
>> +       ssize_t size;
>> +
>> +       if (cd->cur_ptr <= cd->end_of_data)
>> +               size = cd->end_of_data - cd->cur_ptr;
>> +       else
>> +               size = (cd->end_addr - cd->cur_ptr) +
>> +                       (cd->end_of_data - cd->start_addr);
>> +
>> +       return size;
>> +}
>> +
>> +static int dpaa2_generic_console_open(struct inode *node, struct file *fp,
>> +                                     u64 offset, u64 size,
>> +                                     u32 expected_magic,
>> +                                     u32 offset_delta)
>> +{
>> +       u32 read_magic, wrapped, last_byte, buf_start, buf_length;
>> +       struct console_data *cd;
>> +       u64 base_addr;
>> +       int err;
>> +
>> +       cd = kmalloc(sizeof(*cd), GFP_KERNEL);
>> +       if (!cd)
>> +               return -ENOMEM;
>> +
>> +       base_addr = get_mc_fw_base_address();
>> +       if (base_addr < 0)
> 
> We are leaking cd structure here.

Ugh, that's right. Will fix.

> 
>> +               return -EIO;
>> +
>> +       cd->map_addr = ioremap(base_addr + offset, size);
>> +       if (!cd->map_addr) {
>> +               pr_err("cannot map console log memory\n");
>> +               err = -EIO;
>> +               goto err_ioremap;
>> +       }
>> +
>> +       cd->hdr = (struct log_header *)cd->map_addr;
>> +       read_magic = readl(&cd->hdr->magic_word);
>> +       last_byte = readl(&cd->hdr->last_byte);
>> +       buf_start = readl(&cd->hdr->buf_start);
>> +       buf_length = readl(&cd->hdr->buf_length);
>> +
>> +       if (read_magic != expected_magic) {
>> +               pr_warn("expected = %08x, read = %08x\n",
>> +                       expected_magic, read_magic);
>> +               err = -EIO;
>> +               goto err_magic;
>> +       }
>> +
>> +       cd->start_addr = cd->map_addr + buf_start - offset_delta;
>> +       cd->end_addr = cd->start_addr + buf_length;
>> +
>> +       wrapped = last_byte & LOG_HEADER_FLAG_BUFFER_WRAPAROUND;
>> +
>> +       adjust_end(cd);
>> +       if (wrapped && cd->end_of_data != cd->end_addr)
>> +               cd->cur_ptr = cd->end_of_data + 1;
>> +       else
>> +               cd->cur_ptr = cd->start_addr;
>> +
>> +       fp->private_data = cd;
>> +
>> +       return 0;
>> +
>> +err_magic:
>> +       iounmap(cd->map_addr);
>> +
>> +err_ioremap:
>> +       kfree(cd);
>> +
>> +       return err;
>> +}
>> +
>> +static int dpaa2_mc_console_open(struct inode *node, struct file *fp)
>> +{
>> +       return dpaa2_generic_console_open(node, fp,
>> +                                         MC_BUFFER_OFFSET, MC_BUFFER_SIZE,
>> +                                         MAGIC_MC, MC_OFFSET_DELTA);
>> +}
>> +
>> +static int dpaa2_aiop_console_open(struct inode *node, struct file *fp)
>> +{
>> +       return dpaa2_generic_console_open(node, fp,
>> +                                         AIOP_BUFFER_OFFSET, AIOP_BUFFER_SIZE,
>> +                                         MAGIC_AIOP, AIOP_OFFSET_DELTA);
>> +}
>> +
>> +static int dpaa2_console_close(struct inode *node, struct file *fp)
>> +{
>> +       struct console_data *cd = fp->private_data;
>> +
>> +       iounmap(cd->map_addr);
>> +       kfree(cd);
>> +       return 0;
>> +}
>> +
>> +static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
>> +                                 size_t count, loff_t *f_pos)
>> +{
>> +       struct console_data *cd = fp->private_data;
>> +       size_t bytes = dpaa2_console_size(cd);
>> +       size_t bytes_end = cd->end_addr - cd->cur_ptr;
>> +       size_t written = 0;
>> +
>> +       /* Check if we need to adjust the end of data addr */
>> +       adjust_end(cd);
>> +
>> +       if (cd->end_of_data == cd->cur_ptr)
>> +               return 0;
>> +
>> +       if (count < bytes)
>> +               bytes = count;
>> +
>> +       if (bytes > bytes_end) {
>> +               if (copy_to_user(buf, cd->cur_ptr, bytes_end))
>> +                       return -EFAULT;
>> +               buf += bytes_end;
>> +               cd->cur_ptr = cd->start_addr;
>> +               bytes -= bytes_end;
>> +               written += bytes_end;
>> +       }
>> +
>> +       if (copy_to_user(buf, cd->cur_ptr, bytes))
>> +               return -EFAULT;
>> +       cd->cur_ptr += bytes;
>> +       written += bytes;
>> +
>> +       return written;
>> +}
>> +
>> +static const struct file_operations dpaa2_mc_console_fops = {
>> +       .owner          = THIS_MODULE,
>> +       .open           = dpaa2_mc_console_open,
>> +       .release        = dpaa2_console_close,
>> +       .read           = dpaa2_console_read,
>> +};
>> +
>> +static struct miscdevice dpaa2_mc_console_dev = {
>> +       .minor = MISC_DYNAMIC_MINOR,
>> +       .name = "dpaa2_mc_console",
>> +       .fops = &dpaa2_mc_console_fops
>> +};
>> +
>> +static const struct file_operations dpaa2_aiop_console_fops = {
>> +       .owner          = THIS_MODULE,
>> +       .open           = dpaa2_aiop_console_open,
>> +       .release        = dpaa2_console_close,
>> +       .read           = dpaa2_console_read,
>> +};
>> +
>> +static struct miscdevice dpaa2_aiop_console_dev = {
>> +       .minor = MISC_DYNAMIC_MINOR,
>> +       .name = "dpaa2_aiop_console",
>> +       .fops = &dpaa2_aiop_console_fops
>> +};
>> +
>> +static int dpaa2_console_probe(struct platform_device *pdev)
>> +{
>> +       int error;
>> +
>> +       error = of_address_to_resource(pdev->dev.of_node, 0, &mc_base_addr);
>> +       if (error < 0) {
>> +               pr_err("of_address_to_resource() failed for %pOF with %d\n",
>> +                      pdev->dev.of_node, error);
>> +               return error;
>> +       }
>> +
>> +       error = misc_register(&dpaa2_mc_console_dev);
>> +       if (error) {
>> +               pr_err("dpaa2-console: cannot register device %s\n",
>> +                      dpaa2_mc_console_dev.name);
> 
> The pr_fmt already have the dpaa2-console prefix.
> 
>> +               goto err_register_mc;
>> +       }
>> +
>> +       error = misc_register(&dpaa2_aiop_console_dev);
>> +       if (error) {
>> +               pr_err("dpaa2-console: cannot register device %s\n",
>> +                      dpaa2_aiop_console_dev.name);
> 
> The pr_fmt already have the dpaa2-console prefix.
> 


Yep, will remove the duplicate "dpaa2-console" prefix.

Thanks for the review,
Ioana C


>> +               goto err_register_aiop;
>> +       }
>> +
>> +       return 0;
>> +
>> +err_register_aiop:
>> +       misc_deregister(&dpaa2_mc_console_dev);
>> +err_register_mc:
>> +       return error;
>> +}
>> +
>> +static int dpaa2_console_remove(struct platform_device *pdev)
>> +{
>> +       misc_deregister(&dpaa2_mc_console_dev);
>> +       misc_deregister(&dpaa2_aiop_console_dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct of_device_id dpaa2_console_match_table[] = {
>> +       { .compatible = "fsl,dpaa2-console",},
>> +       {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, dpaa2_console_match_table);
>> +
>> +static struct platform_driver dpaa2_console_driver = {
>> +       .driver = {
>> +                  .name = "dpaa2-console",
>> +                  .pm = NULL,
>> +                  .of_match_table = dpaa2_console_match_table,
>> +                  },
>> +       .probe = dpaa2_console_probe,
>> +       .remove = dpaa2_console_remove,
>> +};
>> +module_platform_driver(dpaa2_console_driver);
>> --
>> 1.9.1
>>
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] soc: fsl: add DPAA2 console support
  2019-04-04 10:48   ` Ioana Ciornei
@ 2019-04-04 18:20     ` Leo Li
  2019-04-05  9:05       ` Ioana Ciornei
  0 siblings, 1 reply; 5+ messages in thread
From: Leo Li @ 2019-04-04 18:20 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: linux-kernel, Roy Pledge



> -----Original Message-----
> From: Ioana Ciornei
> Sent: Thursday, April 4, 2019 5:48 AM
> To: Leo Li <leoyang.li@nxp.com>
> Cc: linux-kernel@vger.kernel.org; Roy Pledge <roy.pledge@nxp.com>
> Subject: Re: [PATCH] soc: fsl: add DPAA2 console support
> 
> On 4/3/19 10:55 PM, Li Yang wrote:
> > On Tue, Mar 26, 2019 at 2:17 PM Ioana Ciornei <ioana.ciornei@nxp.com>
> wrote:
> >>
> >> This patch adds DPAA2 MC and AIOP console log support.
> >>
> >> The platform driver probes on the "fsl,dpaa2-console" device tree
> >> node which describes the base firmware address needed in order to
> >> infer the start address of both firmware logs: MC and AIOP.
> >> It then exports two misc char devices which can be used to dump the
> >> needed logs.
> >>
> >> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> >> Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
> >> ---
> >>   drivers/soc/fsl/Kconfig         |  10 ++
> >>   drivers/soc/fsl/Makefile        |   1 +
> >>   drivers/soc/fsl/dpaa2-console.c | 306
> ++++++++++++++++++++++++++++++++++++++++
> >>   3 files changed, 317 insertions(+)
> >>   create mode 100644 drivers/soc/fsl/dpaa2-console.c
> >>
> >> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
> >> 61f8e1433d0a..9fe8222c3968 100644
> >> --- a/drivers/soc/fsl/Kconfig
> >> +++ b/drivers/soc/fsl/Kconfig
> >> @@ -29,4 +29,14 @@ config FSL_MC_DPIO
> >>            other DPAA2 objects. This driver does not expose the DPIO
> >>            objects individually, but groups them under a service layer
> >>            API.
> >> +
> >> +config DPAA2_CONSOLE
> >> +       tristate "QorIQ DPAA2 console driver"
> >> +       depends on OF && (ARCH_LAYERSCAPE || (COMPILE_TEST && (ARM
> ||
> >> +ARM64 || X86_LOCAL_APIC || PPC)))
> >
> > I don't know why FSL_MC driver added there architectures dependency
> > for COMPILE_TEST, but is this driver also only buildable on these
> > architectures?  Can we really just remove these dependencies?
> >
> 
> The initial reasoning behind the dependency on the COMPILE_TEST config
> was to catch early build problems on any build configuration.
> And yes, the dpaa2-console can be compiled without a problem on these
> architectures.
> 
> In this case, I would suggest to leave the dependencies as they are but if you
> want I can also remove them.

Can we keep the COMPILE_TEST and remove the (ARM || ARM64 || X86_LOCAL_APIC || PPC) if we are not using any architecture specific stuff?

> 
> >> +       default y
> >> +       help
> >> +         Console driver for DPAA2 platforms. Exports 2 char devices,
> >> +         /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
> >> +         which can be used to dump the Management Complex and AIOP
> >> +         firmware logs.
> >>   endmenu
> >> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> >> index 803ef1bfb5ff..57762c9fc7da 100644
> >> --- a/drivers/soc/fsl/Makefile
> >> +++ b/drivers/soc/fsl/Makefile
> >> @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)              += qe/
> >>   obj-$(CONFIG_CPM)                      += qe/
> >>   obj-$(CONFIG_FSL_GUTS)                 += guts.o
> >>   obj-$(CONFIG_FSL_MC_DPIO)              += dpio/
> >> +obj-$(CONFIG_DPAA2_CONSOLE)            += dpaa2-console.o
> >> diff --git a/drivers/soc/fsl/dpaa2-console.c
> >> b/drivers/soc/fsl/dpaa2-console.c new file mode 100644 index
> >> 000000000000..21a5e121f87b
> >> --- /dev/null
> >> +++ b/drivers/soc/fsl/dpaa2-console.c
> >> @@ -0,0 +1,306 @@
> >> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> >
> > Probably good to have a one-line description of the driver?
> 
> Will add one in the next version.
> 
> >
> >> +/* Copyright 2015-2016 Freescale Semiconductor Inc.
> >> + * Copyright 2018 NXP
> >> + */
> >> +
> >> +#define pr_fmt(fmt) "dpaa2-console: " fmt
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/of_address.h>
> >> +#include <linux/miscdevice.h>
> >> +#include <linux/uaccess.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/fs.h>
> >> +#include <linux/io.h>
> >> +
> >> +MODULE_LICENSE("Dual BSD/GPL");
> >> +MODULE_AUTHOR("Roy Pledge <roy.pledge@nxp.com>");
> >> +MODULE_DESCRIPTION("DPAA2 console driver");
> >
> > These normally go to the bottom of a driver.
> 
> Will move them.
> 
> >
> >> +
> >> +/* MC firmware base low/high registers indexes */ #define
> >> +MCFBALR_OFFSET 0 #define MCFBAHR_OFFSET 1
> >> +
> >> +/* Bit masks used to get the most/least significant part of the MC
> >> +base addr */ #define MC_FW_ADDR_MASK_HIGH 0x1FFFF #define
> >> +MC_FW_ADDR_MASK_LOW  0xE0000000
> >> +
> >> +#define MC_BUFFER_OFFSET 0x01000000
> >> +#define MC_BUFFER_SIZE   (1024 * 1024 * 16)
> >> +#define MC_OFFSET_DELTA  MC_BUFFER_OFFSET
> >> +
> >> +#define AIOP_BUFFER_OFFSET 0x06000000
> >> +#define AIOP_BUFFER_SIZE   (1024 * 1024 * 16)
> >> +#define AIOP_OFFSET_DELTA  0
> >> +
> >> +#define LOG_HEADER_FLAG_BUFFER_WRAPAROUND 0x80000000
> #define
> >> +LAST_BYTE(a) ((a) & ~(LOG_HEADER_FLAG_BUFFER_WRAPAROUND))
> >> +
> >> +/* MC and AIOP Magic words */
> >> +#define MAGIC_MC   0x4d430100
> >> +#define MAGIC_AIOP 0x41494F50
> >> +
> >> +struct log_header {
> >> +       __le32 magic_word;
> >> +       char reserved[4];
> >> +       __le32 buf_start;
> >> +       __le32 buf_length;
> >> +       __le32 last_byte;
> >> +};
> >> +
> >> +struct console_data {
> >> +       char *map_addr;
> >> +       struct log_header *hdr;
> >> +       char *start_addr;
> >> +       char *end_addr;
> >> +       char *end_of_data;
> >> +       char *cur_ptr;
> >> +};
> >> +
> >> +struct resource mc_base_addr;
> >> +
> >> +static inline void adjust_end(struct console_data *cd) {
> >> +       u32 last_byte = readl(&cd->hdr->last_byte);
> >> +
> >> +       cd->end_of_data = cd->start_addr + LAST_BYTE(last_byte); }
> >> +
> >> +static u64 get_mc_fw_base_address(void) {
> >> +       u64 mcfwbase = 0ULL;
> >> +       u32 *mcfbaregs;
> >> +
> >> +       mcfbaregs = (u32 *)ioremap(mc_base_addr.start,
> >> +                                  resource_size(&mc_base_addr));
> >> +       if (!mcfbaregs) {
> >> +               pr_err("could not map MC Firmaware Base registers\n");
> >> +               return -EIO;
> >> +       }
> >> +
> >> +       mcfwbase  = readl(mcfbaregs + MCFBAHR_OFFSET) &
> MC_FW_ADDR_MASK_HIGH;
> >> +       mcfwbase <<= 32;
> >> +       mcfwbase |= readl(mcfbaregs + MCFBALR_OFFSET) &
> MC_FW_ADDR_MASK_LOW;
> >> +       iounmap(mcfbaregs);
> >> +
> >> +       pr_debug("MC base address at 0x%016llx\n", mcfwbase);
> >> +       return mcfwbase;
> >> +}
> >> +
> >> +static ssize_t dpaa2_console_size(struct console_data *cd) {
> >> +       ssize_t size;
> >> +
> >> +       if (cd->cur_ptr <= cd->end_of_data)
> >> +               size = cd->end_of_data - cd->cur_ptr;
> >> +       else
> >> +               size = (cd->end_addr - cd->cur_ptr) +
> >> +                       (cd->end_of_data - cd->start_addr);
> >> +
> >> +       return size;
> >> +}
> >> +
> >> +static int dpaa2_generic_console_open(struct inode *node, struct file *fp,
> >> +                                     u64 offset, u64 size,
> >> +                                     u32 expected_magic,
> >> +                                     u32 offset_delta) {
> >> +       u32 read_magic, wrapped, last_byte, buf_start, buf_length;
> >> +       struct console_data *cd;
> >> +       u64 base_addr;
> >> +       int err;
> >> +
> >> +       cd = kmalloc(sizeof(*cd), GFP_KERNEL);
> >> +       if (!cd)
> >> +               return -ENOMEM;
> >> +
> >> +       base_addr = get_mc_fw_base_address();
> >> +       if (base_addr < 0)
> >
> > We are leaking cd structure here.
> 
> Ugh, that's right. Will fix.
> 
> >
> >> +               return -EIO;
> >> +
> >> +       cd->map_addr = ioremap(base_addr + offset, size);
> >> +       if (!cd->map_addr) {
> >> +               pr_err("cannot map console log memory\n");
> >> +               err = -EIO;
> >> +               goto err_ioremap;
> >> +       }
> >> +
> >> +       cd->hdr = (struct log_header *)cd->map_addr;
> >> +       read_magic = readl(&cd->hdr->magic_word);
> >> +       last_byte = readl(&cd->hdr->last_byte);
> >> +       buf_start = readl(&cd->hdr->buf_start);
> >> +       buf_length = readl(&cd->hdr->buf_length);
> >> +
> >> +       if (read_magic != expected_magic) {
> >> +               pr_warn("expected = %08x, read = %08x\n",
> >> +                       expected_magic, read_magic);
> >> +               err = -EIO;
> >> +               goto err_magic;
> >> +       }
> >> +
> >> +       cd->start_addr = cd->map_addr + buf_start - offset_delta;
> >> +       cd->end_addr = cd->start_addr + buf_length;
> >> +
> >> +       wrapped = last_byte & LOG_HEADER_FLAG_BUFFER_WRAPAROUND;
> >> +
> >> +       adjust_end(cd);
> >> +       if (wrapped && cd->end_of_data != cd->end_addr)
> >> +               cd->cur_ptr = cd->end_of_data + 1;
> >> +       else
> >> +               cd->cur_ptr = cd->start_addr;
> >> +
> >> +       fp->private_data = cd;
> >> +
> >> +       return 0;
> >> +
> >> +err_magic:
> >> +       iounmap(cd->map_addr);
> >> +
> >> +err_ioremap:
> >> +       kfree(cd);
> >> +
> >> +       return err;
> >> +}
> >> +
> >> +static int dpaa2_mc_console_open(struct inode *node, struct file
> >> +*fp) {
> >> +       return dpaa2_generic_console_open(node, fp,
> >> +                                         MC_BUFFER_OFFSET, MC_BUFFER_SIZE,
> >> +                                         MAGIC_MC, MC_OFFSET_DELTA);
> >> +}
> >> +
> >> +static int dpaa2_aiop_console_open(struct inode *node, struct file
> >> +*fp) {
> >> +       return dpaa2_generic_console_open(node, fp,
> >> +                                         AIOP_BUFFER_OFFSET, AIOP_BUFFER_SIZE,
> >> +                                         MAGIC_AIOP,
> >> +AIOP_OFFSET_DELTA); }
> >> +
> >> +static int dpaa2_console_close(struct inode *node, struct file *fp)
> >> +{
> >> +       struct console_data *cd = fp->private_data;
> >> +
> >> +       iounmap(cd->map_addr);
> >> +       kfree(cd);
> >> +       return 0;
> >> +}
> >> +
> >> +static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
> >> +                                 size_t count, loff_t *f_pos) {
> >> +       struct console_data *cd = fp->private_data;
> >> +       size_t bytes = dpaa2_console_size(cd);
> >> +       size_t bytes_end = cd->end_addr - cd->cur_ptr;
> >> +       size_t written = 0;
> >> +
> >> +       /* Check if we need to adjust the end of data addr */
> >> +       adjust_end(cd);
> >> +
> >> +       if (cd->end_of_data == cd->cur_ptr)
> >> +               return 0;
> >> +
> >> +       if (count < bytes)
> >> +               bytes = count;
> >> +
> >> +       if (bytes > bytes_end) {
> >> +               if (copy_to_user(buf, cd->cur_ptr, bytes_end))
> >> +                       return -EFAULT;
> >> +               buf += bytes_end;
> >> +               cd->cur_ptr = cd->start_addr;
> >> +               bytes -= bytes_end;
> >> +               written += bytes_end;
> >> +       }
> >> +
> >> +       if (copy_to_user(buf, cd->cur_ptr, bytes))
> >> +               return -EFAULT;
> >> +       cd->cur_ptr += bytes;
> >> +       written += bytes;
> >> +
> >> +       return written;
> >> +}
> >> +
> >> +static const struct file_operations dpaa2_mc_console_fops = {
> >> +       .owner          = THIS_MODULE,
> >> +       .open           = dpaa2_mc_console_open,
> >> +       .release        = dpaa2_console_close,
> >> +       .read           = dpaa2_console_read,
> >> +};
> >> +
> >> +static struct miscdevice dpaa2_mc_console_dev = {
> >> +       .minor = MISC_DYNAMIC_MINOR,
> >> +       .name = "dpaa2_mc_console",
> >> +       .fops = &dpaa2_mc_console_fops };
> >> +
> >> +static const struct file_operations dpaa2_aiop_console_fops = {
> >> +       .owner          = THIS_MODULE,
> >> +       .open           = dpaa2_aiop_console_open,
> >> +       .release        = dpaa2_console_close,
> >> +       .read           = dpaa2_console_read,
> >> +};
> >> +
> >> +static struct miscdevice dpaa2_aiop_console_dev = {
> >> +       .minor = MISC_DYNAMIC_MINOR,
> >> +       .name = "dpaa2_aiop_console",
> >> +       .fops = &dpaa2_aiop_console_fops };
> >> +
> >> +static int dpaa2_console_probe(struct platform_device *pdev) {
> >> +       int error;
> >> +
> >> +       error = of_address_to_resource(pdev->dev.of_node, 0,
> &mc_base_addr);
> >> +       if (error < 0) {
> >> +               pr_err("of_address_to_resource() failed for %pOF with %d\n",
> >> +                      pdev->dev.of_node, error);
> >> +               return error;
> >> +       }
> >> +
> >> +       error = misc_register(&dpaa2_mc_console_dev);
> >> +       if (error) {
> >> +               pr_err("dpaa2-console: cannot register device %s\n",
> >> +                      dpaa2_mc_console_dev.name);
> >
> > The pr_fmt already have the dpaa2-console prefix.
> >
> >> +               goto err_register_mc;
> >> +       }
> >> +
> >> +       error = misc_register(&dpaa2_aiop_console_dev);
> >> +       if (error) {
> >> +               pr_err("dpaa2-console: cannot register device %s\n",
> >> +                      dpaa2_aiop_console_dev.name);
> >
> > The pr_fmt already have the dpaa2-console prefix.
> >
> 
> 
> Yep, will remove the duplicate "dpaa2-console" prefix.
> 
> Thanks for the review,
> Ioana C
> 
> 
> >> +               goto err_register_aiop;
> >> +       }
> >> +
> >> +       return 0;
> >> +
> >> +err_register_aiop:
> >> +       misc_deregister(&dpaa2_mc_console_dev);
> >> +err_register_mc:
> >> +       return error;
> >> +}
> >> +
> >> +static int dpaa2_console_remove(struct platform_device *pdev) {
> >> +       misc_deregister(&dpaa2_mc_console_dev);
> >> +       misc_deregister(&dpaa2_aiop_console_dev);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct of_device_id dpaa2_console_match_table[] = {
> >> +       { .compatible = "fsl,dpaa2-console",},
> >> +       {},
> >> +};
> >> +
> >> +MODULE_DEVICE_TABLE(of, dpaa2_console_match_table);
> >> +
> >> +static struct platform_driver dpaa2_console_driver = {
> >> +       .driver = {
> >> +                  .name = "dpaa2-console",
> >> +                  .pm = NULL,
> >> +                  .of_match_table = dpaa2_console_match_table,
> >> +                  },
> >> +       .probe = dpaa2_console_probe,
> >> +       .remove = dpaa2_console_remove, };
> >> +module_platform_driver(dpaa2_console_driver);
> >> --
> >> 1.9.1
> >>
> >


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] soc: fsl: add DPAA2 console support
  2019-04-04 18:20     ` Leo Li
@ 2019-04-05  9:05       ` Ioana Ciornei
  0 siblings, 0 replies; 5+ messages in thread
From: Ioana Ciornei @ 2019-04-05  9:05 UTC (permalink / raw)
  To: Leo Li; +Cc: linux-kernel, Roy Pledge

On 4/4/19 9:20 PM, Leo Li wrote:
> 
> 
>> -----Original Message-----
>> From: Ioana Ciornei
>> Sent: Thursday, April 4, 2019 5:48 AM
>> To: Leo Li <leoyang.li@nxp.com>
>> Cc: linux-kernel@vger.kernel.org; Roy Pledge <roy.pledge@nxp.com>
>> Subject: Re: [PATCH] soc: fsl: add DPAA2 console support
>>
>> On 4/3/19 10:55 PM, Li Yang wrote:
>>> On Tue, Mar 26, 2019 at 2:17 PM Ioana Ciornei <ioana.ciornei@nxp.com>
>> wrote:
>>>>
>>>> This patch adds DPAA2 MC and AIOP console log support.
>>>>
>>>> The platform driver probes on the "fsl,dpaa2-console" device tree
>>>> node which describes the base firmware address needed in order to
>>>> infer the start address of both firmware logs: MC and AIOP.
>>>> It then exports two misc char devices which can be used to dump the
>>>> needed logs.
>>>>
>>>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>>>> Signed-off-by: Roy Pledge <roy.pledge@nxp.com>
>>>> ---
>>>>    drivers/soc/fsl/Kconfig         |  10 ++
>>>>    drivers/soc/fsl/Makefile        |   1 +
>>>>    drivers/soc/fsl/dpaa2-console.c | 306
>> ++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 317 insertions(+)
>>>>    create mode 100644 drivers/soc/fsl/dpaa2-console.c
>>>>
>>>> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
>>>> 61f8e1433d0a..9fe8222c3968 100644
>>>> --- a/drivers/soc/fsl/Kconfig
>>>> +++ b/drivers/soc/fsl/Kconfig
>>>> @@ -29,4 +29,14 @@ config FSL_MC_DPIO
>>>>             other DPAA2 objects. This driver does not expose the DPIO
>>>>             objects individually, but groups them under a service layer
>>>>             API.
>>>> +
>>>> +config DPAA2_CONSOLE
>>>> +       tristate "QorIQ DPAA2 console driver"
>>>> +       depends on OF && (ARCH_LAYERSCAPE || (COMPILE_TEST && (ARM
>> ||
>>>> +ARM64 || X86_LOCAL_APIC || PPC)))
>>>
>>> I don't know why FSL_MC driver added there architectures dependency
>>> for COMPILE_TEST, but is this driver also only buildable on these
>>> architectures?  Can we really just remove these dependencies?
>>>
>>
>> The initial reasoning behind the dependency on the COMPILE_TEST config
>> was to catch early build problems on any build configuration.
>> And yes, the dpaa2-console can be compiled without a problem on these
>> architectures.
>>
>> In this case, I would suggest to leave the dependencies as they are but if you
>> want I can also remove them.
> 
> Can we keep the COMPILE_TEST and remove the (ARM || ARM64 || X86_LOCAL_APIC || PPC) if we are not using any architecture specific stuff?

Yes, that should be a better solution. Will send a v2 with all the changes.

Thanks,
Ioana C


> 
>>
>>>> +       default y
>>>> +       help
>>>> +         Console driver for DPAA2 platforms. Exports 2 char devices,
>>>> +         /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
>>>> +         which can be used to dump the Management Complex and AIOP
>>>> +         firmware logs.
>>>>    endmenu
>>>> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
>>>> index 803ef1bfb5ff..57762c9fc7da 100644
>>>> --- a/drivers/soc/fsl/Makefile
>>>> +++ b/drivers/soc/fsl/Makefile
>>>> @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)              += qe/
>>>>    obj-$(CONFIG_CPM)                      += qe/
>>>>    obj-$(CONFIG_FSL_GUTS)                 += guts.o
>>>>    obj-$(CONFIG_FSL_MC_DPIO)              += dpio/
>>>> +obj-$(CONFIG_DPAA2_CONSOLE)            += dpaa2-console.o
>>>> diff --git a/drivers/soc/fsl/dpaa2-console.c
>>>> b/drivers/soc/fsl/dpaa2-console.c new file mode 100644 index
>>>> 000000000000..21a5e121f87b
>>>> --- /dev/null
>>>> +++ b/drivers/soc/fsl/dpaa2-console.c
>>>> @@ -0,0 +1,306 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>>>
>>> Probably good to have a one-line description of the driver?
>>
>> Will add one in the next version.
>>
>>>
>>>> +/* Copyright 2015-2016 Freescale Semiconductor Inc.
>>>> + * Copyright 2018 NXP
>>>> + */
>>>> +
>>>> +#define pr_fmt(fmt) "dpaa2-console: " fmt
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_device.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/miscdevice.h>
>>>> +#include <linux/uaccess.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/fs.h>
>>>> +#include <linux/io.h>
>>>> +
>>>> +MODULE_LICENSE("Dual BSD/GPL");
>>>> +MODULE_AUTHOR("Roy Pledge <roy.pledge@nxp.com>");
>>>> +MODULE_DESCRIPTION("DPAA2 console driver");
>>>
>>> These normally go to the bottom of a driver.
>>
>> Will move them.
>>
>>>
>>>> +
>>>> +/* MC firmware base low/high registers indexes */ #define
>>>> +MCFBALR_OFFSET 0 #define MCFBAHR_OFFSET 1
>>>> +
>>>> +/* Bit masks used to get the most/least significant part of the MC
>>>> +base addr */ #define MC_FW_ADDR_MASK_HIGH 0x1FFFF #define
>>>> +MC_FW_ADDR_MASK_LOW  0xE0000000
>>>> +
>>>> +#define MC_BUFFER_OFFSET 0x01000000
>>>> +#define MC_BUFFER_SIZE   (1024 * 1024 * 16)
>>>> +#define MC_OFFSET_DELTA  MC_BUFFER_OFFSET
>>>> +
>>>> +#define AIOP_BUFFER_OFFSET 0x06000000
>>>> +#define AIOP_BUFFER_SIZE   (1024 * 1024 * 16)
>>>> +#define AIOP_OFFSET_DELTA  0
>>>> +
>>>> +#define LOG_HEADER_FLAG_BUFFER_WRAPAROUND 0x80000000
>> #define
>>>> +LAST_BYTE(a) ((a) & ~(LOG_HEADER_FLAG_BUFFER_WRAPAROUND))
>>>> +
>>>> +/* MC and AIOP Magic words */
>>>> +#define MAGIC_MC   0x4d430100
>>>> +#define MAGIC_AIOP 0x41494F50
>>>> +
>>>> +struct log_header {
>>>> +       __le32 magic_word;
>>>> +       char reserved[4];
>>>> +       __le32 buf_start;
>>>> +       __le32 buf_length;
>>>> +       __le32 last_byte;
>>>> +};
>>>> +
>>>> +struct console_data {
>>>> +       char *map_addr;
>>>> +       struct log_header *hdr;
>>>> +       char *start_addr;
>>>> +       char *end_addr;
>>>> +       char *end_of_data;
>>>> +       char *cur_ptr;
>>>> +};
>>>> +
>>>> +struct resource mc_base_addr;
>>>> +
>>>> +static inline void adjust_end(struct console_data *cd) {
>>>> +       u32 last_byte = readl(&cd->hdr->last_byte);
>>>> +
>>>> +       cd->end_of_data = cd->start_addr + LAST_BYTE(last_byte); }
>>>> +
>>>> +static u64 get_mc_fw_base_address(void) {
>>>> +       u64 mcfwbase = 0ULL;
>>>> +       u32 *mcfbaregs;
>>>> +
>>>> +       mcfbaregs = (u32 *)ioremap(mc_base_addr.start,
>>>> +                                  resource_size(&mc_base_addr));
>>>> +       if (!mcfbaregs) {
>>>> +               pr_err("could not map MC Firmaware Base registers\n");
>>>> +               return -EIO;
>>>> +       }
>>>> +
>>>> +       mcfwbase  = readl(mcfbaregs + MCFBAHR_OFFSET) &
>> MC_FW_ADDR_MASK_HIGH;
>>>> +       mcfwbase <<= 32;
>>>> +       mcfwbase |= readl(mcfbaregs + MCFBALR_OFFSET) &
>> MC_FW_ADDR_MASK_LOW;
>>>> +       iounmap(mcfbaregs);
>>>> +
>>>> +       pr_debug("MC base address at 0x%016llx\n", mcfwbase);
>>>> +       return mcfwbase;
>>>> +}
>>>> +
>>>> +static ssize_t dpaa2_console_size(struct console_data *cd) {
>>>> +       ssize_t size;
>>>> +
>>>> +       if (cd->cur_ptr <= cd->end_of_data)
>>>> +               size = cd->end_of_data - cd->cur_ptr;
>>>> +       else
>>>> +               size = (cd->end_addr - cd->cur_ptr) +
>>>> +                       (cd->end_of_data - cd->start_addr);
>>>> +
>>>> +       return size;
>>>> +}
>>>> +
>>>> +static int dpaa2_generic_console_open(struct inode *node, struct file *fp,
>>>> +                                     u64 offset, u64 size,
>>>> +                                     u32 expected_magic,
>>>> +                                     u32 offset_delta) {
>>>> +       u32 read_magic, wrapped, last_byte, buf_start, buf_length;
>>>> +       struct console_data *cd;
>>>> +       u64 base_addr;
>>>> +       int err;
>>>> +
>>>> +       cd = kmalloc(sizeof(*cd), GFP_KERNEL);
>>>> +       if (!cd)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       base_addr = get_mc_fw_base_address();
>>>> +       if (base_addr < 0)
>>>
>>> We are leaking cd structure here.
>>
>> Ugh, that's right. Will fix.
>>
>>>
>>>> +               return -EIO;
>>>> +
>>>> +       cd->map_addr = ioremap(base_addr + offset, size);
>>>> +       if (!cd->map_addr) {
>>>> +               pr_err("cannot map console log memory\n");
>>>> +               err = -EIO;
>>>> +               goto err_ioremap;
>>>> +       }
>>>> +
>>>> +       cd->hdr = (struct log_header *)cd->map_addr;
>>>> +       read_magic = readl(&cd->hdr->magic_word);
>>>> +       last_byte = readl(&cd->hdr->last_byte);
>>>> +       buf_start = readl(&cd->hdr->buf_start);
>>>> +       buf_length = readl(&cd->hdr->buf_length);
>>>> +
>>>> +       if (read_magic != expected_magic) {
>>>> +               pr_warn("expected = %08x, read = %08x\n",
>>>> +                       expected_magic, read_magic);
>>>> +               err = -EIO;
>>>> +               goto err_magic;
>>>> +       }
>>>> +
>>>> +       cd->start_addr = cd->map_addr + buf_start - offset_delta;
>>>> +       cd->end_addr = cd->start_addr + buf_length;
>>>> +
>>>> +       wrapped = last_byte & LOG_HEADER_FLAG_BUFFER_WRAPAROUND;
>>>> +
>>>> +       adjust_end(cd);
>>>> +       if (wrapped && cd->end_of_data != cd->end_addr)
>>>> +               cd->cur_ptr = cd->end_of_data + 1;
>>>> +       else
>>>> +               cd->cur_ptr = cd->start_addr;
>>>> +
>>>> +       fp->private_data = cd;
>>>> +
>>>> +       return 0;
>>>> +
>>>> +err_magic:
>>>> +       iounmap(cd->map_addr);
>>>> +
>>>> +err_ioremap:
>>>> +       kfree(cd);
>>>> +
>>>> +       return err;
>>>> +}
>>>> +
>>>> +static int dpaa2_mc_console_open(struct inode *node, struct file
>>>> +*fp) {
>>>> +       return dpaa2_generic_console_open(node, fp,
>>>> +                                         MC_BUFFER_OFFSET, MC_BUFFER_SIZE,
>>>> +                                         MAGIC_MC, MC_OFFSET_DELTA);
>>>> +}
>>>> +
>>>> +static int dpaa2_aiop_console_open(struct inode *node, struct file
>>>> +*fp) {
>>>> +       return dpaa2_generic_console_open(node, fp,
>>>> +                                         AIOP_BUFFER_OFFSET, AIOP_BUFFER_SIZE,
>>>> +                                         MAGIC_AIOP,
>>>> +AIOP_OFFSET_DELTA); }
>>>> +
>>>> +static int dpaa2_console_close(struct inode *node, struct file *fp)
>>>> +{
>>>> +       struct console_data *cd = fp->private_data;
>>>> +
>>>> +       iounmap(cd->map_addr);
>>>> +       kfree(cd);
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static ssize_t dpaa2_console_read(struct file *fp, char __user *buf,
>>>> +                                 size_t count, loff_t *f_pos) {
>>>> +       struct console_data *cd = fp->private_data;
>>>> +       size_t bytes = dpaa2_console_size(cd);
>>>> +       size_t bytes_end = cd->end_addr - cd->cur_ptr;
>>>> +       size_t written = 0;
>>>> +
>>>> +       /* Check if we need to adjust the end of data addr */
>>>> +       adjust_end(cd);
>>>> +
>>>> +       if (cd->end_of_data == cd->cur_ptr)
>>>> +               return 0;
>>>> +
>>>> +       if (count < bytes)
>>>> +               bytes = count;
>>>> +
>>>> +       if (bytes > bytes_end) {
>>>> +               if (copy_to_user(buf, cd->cur_ptr, bytes_end))
>>>> +                       return -EFAULT;
>>>> +               buf += bytes_end;
>>>> +               cd->cur_ptr = cd->start_addr;
>>>> +               bytes -= bytes_end;
>>>> +               written += bytes_end;
>>>> +       }
>>>> +
>>>> +       if (copy_to_user(buf, cd->cur_ptr, bytes))
>>>> +               return -EFAULT;
>>>> +       cd->cur_ptr += bytes;
>>>> +       written += bytes;
>>>> +
>>>> +       return written;
>>>> +}
>>>> +
>>>> +static const struct file_operations dpaa2_mc_console_fops = {
>>>> +       .owner          = THIS_MODULE,
>>>> +       .open           = dpaa2_mc_console_open,
>>>> +       .release        = dpaa2_console_close,
>>>> +       .read           = dpaa2_console_read,
>>>> +};
>>>> +
>>>> +static struct miscdevice dpaa2_mc_console_dev = {
>>>> +       .minor = MISC_DYNAMIC_MINOR,
>>>> +       .name = "dpaa2_mc_console",
>>>> +       .fops = &dpaa2_mc_console_fops };
>>>> +
>>>> +static const struct file_operations dpaa2_aiop_console_fops = {
>>>> +       .owner          = THIS_MODULE,
>>>> +       .open           = dpaa2_aiop_console_open,
>>>> +       .release        = dpaa2_console_close,
>>>> +       .read           = dpaa2_console_read,
>>>> +};
>>>> +
>>>> +static struct miscdevice dpaa2_aiop_console_dev = {
>>>> +       .minor = MISC_DYNAMIC_MINOR,
>>>> +       .name = "dpaa2_aiop_console",
>>>> +       .fops = &dpaa2_aiop_console_fops };
>>>> +
>>>> +static int dpaa2_console_probe(struct platform_device *pdev) {
>>>> +       int error;
>>>> +
>>>> +       error = of_address_to_resource(pdev->dev.of_node, 0,
>> &mc_base_addr);
>>>> +       if (error < 0) {
>>>> +               pr_err("of_address_to_resource() failed for %pOF with %d\n",
>>>> +                      pdev->dev.of_node, error);
>>>> +               return error;
>>>> +       }
>>>> +
>>>> +       error = misc_register(&dpaa2_mc_console_dev);
>>>> +       if (error) {
>>>> +               pr_err("dpaa2-console: cannot register device %s\n",
>>>> +                      dpaa2_mc_console_dev.name);
>>>
>>> The pr_fmt already have the dpaa2-console prefix.
>>>
>>>> +               goto err_register_mc;
>>>> +       }
>>>> +
>>>> +       error = misc_register(&dpaa2_aiop_console_dev);
>>>> +       if (error) {
>>>> +               pr_err("dpaa2-console: cannot register device %s\n",
>>>> +                      dpaa2_aiop_console_dev.name);
>>>
>>> The pr_fmt already have the dpaa2-console prefix.
>>>
>>
>>
>> Yep, will remove the duplicate "dpaa2-console" prefix.
>>
>> Thanks for the review,
>> Ioana C
>>
>>
>>>> +               goto err_register_aiop;
>>>> +       }
>>>> +
>>>> +       return 0;
>>>> +
>>>> +err_register_aiop:
>>>> +       misc_deregister(&dpaa2_mc_console_dev);
>>>> +err_register_mc:
>>>> +       return error;
>>>> +}
>>>> +
>>>> +static int dpaa2_console_remove(struct platform_device *pdev) {
>>>> +       misc_deregister(&dpaa2_mc_console_dev);
>>>> +       misc_deregister(&dpaa2_aiop_console_dev);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id dpaa2_console_match_table[] = {
>>>> +       { .compatible = "fsl,dpaa2-console",},
>>>> +       {},
>>>> +};
>>>> +
>>>> +MODULE_DEVICE_TABLE(of, dpaa2_console_match_table);
>>>> +
>>>> +static struct platform_driver dpaa2_console_driver = {
>>>> +       .driver = {
>>>> +                  .name = "dpaa2-console",
>>>> +                  .pm = NULL,
>>>> +                  .of_match_table = dpaa2_console_match_table,
>>>> +                  },
>>>> +       .probe = dpaa2_console_probe,
>>>> +       .remove = dpaa2_console_remove, };
>>>> +module_platform_driver(dpaa2_console_driver);
>>>> --
>>>> 1.9.1
>>>>
>>>
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-04-05  9:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26 19:16 [PATCH] soc: fsl: add DPAA2 console support Ioana Ciornei
2019-04-03 19:55 ` Li Yang
2019-04-04 10:48   ` Ioana Ciornei
2019-04-04 18:20     ` Leo Li
2019-04-05  9:05       ` Ioana Ciornei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.