On 4/3/19 10:55 PM, Li Yang wrote: > On Tue, Mar 26, 2019 at 2:17 PM Ioana Ciornei <ioana.cior...@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.cior...@nxp.com> >> Signed-off-by: Roy Pledge <roy.ple...@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.ple...@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 >> >