* [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver
@ 2019-12-09 14:23 shubhrajyoti.datta
2019-12-09 14:23 ` [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM shubhrajyoti.datta
2019-12-09 15:30 ` [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver Greg KH
0 siblings, 2 replies; 5+ messages in thread
From: shubhrajyoti.datta @ 2019-12-09 14:23 UTC (permalink / raw)
To: linux-kernel, devicetree
Cc: michal.simek, gregkh, robh+dt, Shubhrajyoti Datta,
Subbaraya Sundeep Bhatta
From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Added driver for Xilinx AXI performance monitor IP.
Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v2:
Updated the header
drivers/uio/Kconfig | 12 ++
drivers/uio/Makefile | 1 +
drivers/uio/uio_xilinx_apm.c | 358 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 371 insertions(+)
create mode 100644 drivers/uio/uio_xilinx_apm.c
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 202ee81..de30312 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -165,4 +165,16 @@ config UIO_HV_GENERIC
to network and storage devices from userspace.
If you compile this as a module, it will be called uio_hv_generic.
+
+config UIO_XILINX_APM
+ tristate "Xilinx AXI Performance Monitor driver"
+ depends on MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP
+ help
+ This driver is developed for AXI Performance Monitor IP, designed to
+ monitor AXI4 traffic for performance analysis of AXI bus in the
+ system. Driver maps HW registers and parameters to userspace.
+
+ To compile this driver as a module, choose M here; the module
+ will be called uio_xilinx_apm.
+
endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index c285dd2..b3464d8 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
obj-$(CONFIG_UIO_MF624) += uio_mf624.o
obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
+obj-$(CONFIG_UIO_XILINX_APM) += uio_xilinx_apm.o
diff --git a/drivers/uio/uio_xilinx_apm.c b/drivers/uio/uio_xilinx_apm.c
new file mode 100644
index 0000000..3f69922
--- /dev/null
+++ b/drivers/uio/uio_xilinx_apm.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx AXI Performance Monitor
+ *
+ * Description:
+ * This driver is developed for AXI Performance Monitor IP,
+ * designed to monitor AXI4 traffic for performance analysis
+ * of AXI bus in the system. Driver maps HW registers and parameters
+ * to userspace. Userspace need not clear the interrupt of IP since
+ * driver clears the interrupt.
+ *
+ * Copyright (c) 2019 Xilinx Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/uio_driver.h>
+
+#define XAPM_IS_OFFSET 0x0038 /* Interrupt Status Register */
+#define DRV_NAME "xilinxapm_uio"
+#define DRV_VERSION "1.0"
+#define UIO_DUMMY_MEMSIZE 4096
+#define XAPM_MODE_ADVANCED 1
+#define XAPM_MODE_PROFILE 2
+#define XAPM_MODE_TRACE 3
+
+/**
+ * struct xapm_param - HW parameters structure
+ * @mode: Mode in which APM is working
+ * @maxslots: Maximum number of Slots in APM
+ * @eventcnt: Event counting enabled in APM
+ * @eventlog: Event logging enabled in APM
+ * @sampledcnt: Sampled metric counters enabled in APM
+ * @numcounters: Number of counters in APM
+ * @metricwidth: Metric Counter width (32/64)
+ * @sampledwidth: Sampled metric counter width
+ * @globalcntwidth: Global Clock counter width
+ * @scalefactor: Scaling factor
+ * @isr: Interrupts info shared to userspace
+ * @is_32bit_filter: Flags for 32bit filter
+ * @clk: Clock handle
+ */
+struct xapm_param {
+ u32 mode;
+ u32 maxslots;
+ u32 eventcnt;
+ u32 eventlog;
+ u32 sampledcnt;
+ u32 numcounters;
+ u32 metricwidth;
+ u32 sampledwidth;
+ u32 globalcntwidth;
+ u32 scalefactor;
+ u32 isr;
+ bool is_32bit_filter;
+ struct clk *clk;
+};
+
+/**
+ * struct xapm_dev - Global driver structure
+ * @info: uio_info structure
+ * @param: xapm_param structure
+ * @regs: IOmapped base address
+ */
+struct xapm_dev {
+ struct uio_info info;
+ struct xapm_param param;
+ void __iomem *regs;
+};
+
+/**
+ * xapm_handler - Interrupt handler for APM
+ * @irq: IRQ number
+ * @info: Pointer to uio_info structure
+ *
+ * Return: Always returns IRQ_HANDLED
+ */
+static irqreturn_t xapm_handler(int irq, struct uio_info *info)
+{
+ struct xapm_dev *xapm = (struct xapm_dev *)info->priv;
+ void *ptr;
+
+ ptr = (unsigned long *)xapm->info.mem[1].addr;
+ /* Clear the interrupt and copy the ISR value to userspace */
+ xapm->param.isr = readl(xapm->regs + XAPM_IS_OFFSET);
+ writel(xapm->param.isr, xapm->regs + XAPM_IS_OFFSET);
+ memcpy(ptr, &xapm->param, sizeof(struct xapm_param));
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * xapm_getprop - Retrieves dts properties to param structure
+ * @pdev: Pointer to platform device
+ * @param: Pointer to param structure
+ *
+ * Returns: '0' on success and failure value on error
+ */
+static int xapm_getprop(struct platform_device *pdev, struct xapm_param *param)
+{
+ u32 mode = 0;
+ int ret;
+ struct device_node *node;
+
+ node = pdev->dev.of_node;
+
+ /* Retrieve required dts properties and fill param structure */
+ ret = of_property_read_u32(node, "xlnx,enable-profile", &mode);
+ if (ret < 0)
+ dev_info(&pdev->dev, "no property xlnx,enable-profile\n");
+ else if (mode)
+ param->mode = XAPM_MODE_PROFILE;
+
+ ret = of_property_read_u32(node, "xlnx,enable-trace", &mode);
+ if (ret < 0)
+ dev_info(&pdev->dev, "no property xlnx,enable-trace\n");
+ else if (mode)
+ param->mode = XAPM_MODE_TRACE;
+
+ ret = of_property_read_u32(node, "xlnx,num-monitor-slots",
+ ¶m->maxslots);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,num-monitor-slots");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,enable-event-count",
+ ¶m->eventcnt);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,enable-event-count");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,enable-event-log",
+ ¶m->eventlog);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,enable-event-log");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,have-sampled-metric-cnt",
+ ¶m->sampledcnt);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,have-sampled-metric-cnt");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,num-of-counters",
+ ¶m->numcounters);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,num-of-counters");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,metric-count-width",
+ ¶m->metricwidth);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,metric-count-width");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,metrics-sample-count-width",
+ ¶m->sampledwidth);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property metrics-sample-count-width");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,global-count-width",
+ ¶m->globalcntwidth);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,global-count-width");
+ return ret;
+ }
+
+ ret = of_property_read_u32(node, "xlnx,metric-count-scale",
+ ¶m->scalefactor);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "no property xlnx,metric-count-scale");
+ return ret;
+ }
+
+ param->is_32bit_filter = of_property_read_bool(node,
+ "xlnx,id-filter-32bit");
+
+ return 0;
+}
+
+/**
+ * xapm_probe - Driver probe function
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Returns: '0' on success and failure value on error
+ */
+
+static int xapm_probe(struct platform_device *pdev)
+{
+ struct xapm_dev *xapm;
+ struct resource *res;
+ int irq;
+ int ret;
+ void *ptr;
+
+ xapm = devm_kzalloc(&pdev->dev, (sizeof(struct xapm_dev)), GFP_KERNEL);
+ if (!xapm)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ xapm->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(xapm->regs)) {
+ dev_err(&pdev->dev, "unable to iomap registers\n");
+ return PTR_ERR(xapm->regs);
+ }
+
+ xapm->param.clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(xapm->param.clk)) {
+ if (PTR_ERR(xapm->param.clk) != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "axi clock error\n");
+ return PTR_ERR(xapm->param.clk);
+ }
+
+ ret = clk_prepare_enable(xapm->param.clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable clock.\n");
+ return ret;
+ }
+ pm_runtime_get_noresume(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ /* Initialize mode as Advanced so that if no mode in dts, default
+ * is Advanced
+ */
+ xapm->param.mode = XAPM_MODE_ADVANCED;
+ ret = xapm_getprop(pdev, &xapm->param);
+ if (ret < 0)
+ goto err_clk_dis;
+
+ xapm->info.mem[0].name = "xilinx_apm";
+ xapm->info.mem[0].addr = res->start;
+ xapm->info.mem[0].size = resource_size(res);
+ xapm->info.mem[0].memtype = UIO_MEM_PHYS;
+
+ xapm->info.mem[1].addr = (unsigned long)kzalloc(UIO_DUMMY_MEMSIZE,
+ GFP_KERNEL);
+ ptr = (unsigned long *)xapm->info.mem[1].addr;
+ xapm->info.mem[1].size = UIO_DUMMY_MEMSIZE;
+ xapm->info.mem[1].memtype = UIO_MEM_LOGICAL;
+
+ xapm->info.name = "axi-pmon";
+ xapm->info.version = DRV_VERSION;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "unable to get irq\n");
+ ret = irq;
+ goto err_clk_dis;
+ }
+
+ xapm->info.irq = irq;
+ xapm->info.handler = xapm_handler;
+ xapm->info.priv = xapm;
+ xapm->info.irq_flags = IRQF_SHARED;
+
+ memcpy(ptr, &xapm->param, sizeof(struct xapm_param));
+
+ ret = uio_register_device(&pdev->dev, &xapm->info);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "unable to register to UIO\n");
+ goto err_clk_dis;
+ }
+
+ platform_set_drvdata(pdev, xapm);
+
+ dev_info(&pdev->dev, "Probed Xilinx APM\n");
+
+ return 0;
+
+err_clk_dis:
+ clk_disable_unprepare(xapm->param.clk);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ return ret;
+}
+
+/**
+ * xapm_remove - Driver remove function
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Return: Always returns '0'
+ */
+static int xapm_remove(struct platform_device *pdev)
+{
+ struct xapm_dev *xapm = platform_get_drvdata(pdev);
+
+ uio_unregister_device(&xapm->info);
+ clk_disable_unprepare(xapm->param.clk);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+
+ return 0;
+}
+
+static int __maybe_unused xapm_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct xapm_dev *xapm = platform_get_drvdata(pdev);
+
+ clk_disable_unprepare(xapm->param.clk);
+ return 0;
+};
+
+static int __maybe_unused xapm_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct xapm_dev *xapm = platform_get_drvdata(pdev);
+ int ret;
+
+ ret = clk_prepare_enable(xapm->param.clk);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to enable clock.\n");
+ return ret;
+ }
+ return 0;
+};
+
+static const struct dev_pm_ops xapm_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(xapm_runtime_suspend, xapm_runtime_resume)
+ SET_RUNTIME_PM_OPS(xapm_runtime_suspend,
+ xapm_runtime_resume, NULL)
+};
+
+static const struct of_device_id xapm_of_match[] = {
+ { .compatible = "xlnx,axi-perf-monitor", },
+ { /* end of table*/ }
+};
+
+MODULE_DEVICE_TABLE(of, xapm_of_match);
+
+static struct platform_driver xapm_driver = {
+ .driver = {
+ .name = "xilinx-axipmon",
+ .of_match_table = xapm_of_match,
+ .pm = &xapm_dev_pm_ops,
+ },
+ .probe = xapm_probe,
+ .remove = xapm_remove,
+};
+
+module_platform_driver(xapm_driver);
+
+MODULE_AUTHOR("Xilinx Inc.");
+MODULE_DESCRIPTION("Xilinx AXI Performance Monitor driver");
+MODULE_LICENSE("GPL v2");
--
2.1.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM
2019-12-09 14:23 [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver shubhrajyoti.datta
@ 2019-12-09 14:23 ` shubhrajyoti.datta
2019-12-10 11:53 ` Michal Simek
2019-12-19 18:43 ` Rob Herring
2019-12-09 15:30 ` [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver Greg KH
1 sibling, 2 replies; 5+ messages in thread
From: shubhrajyoti.datta @ 2019-12-09 14:23 UTC (permalink / raw)
To: linux-kernel, devicetree
Cc: michal.simek, gregkh, robh+dt, Shubhrajyoti Datta
From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Add the devicetree binding for xilinx APM.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v2:
patch added
.../devicetree/bindings/perf/xilinx_apm.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/perf/xilinx_apm.txt
diff --git a/Documentation/devicetree/bindings/perf/xilinx_apm.txt b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
new file mode 100644
index 0000000..a11c82e
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
@@ -0,0 +1,44 @@
+* Xilinx AXI Performance monitor IP
+
+Required properties:
+- compatible: "xlnx,axi-perf-monitor"
+- interrupts: Should contain APM interrupts.
+- interrupt-parent: Must be core interrupt controller.
+- reg: Should contain APM registers location and length.
+- xlnx,enable-profile: Enables the profile mode.
+- xlnx,enable-trace: Enables trace mode.
+- xlnx,num-monitor-slots: Maximum number of slots in APM.
+- xlnx,enable-event-count: Enable event count.
+- xlnx,enable-event-log: Enable event logging.
+- xlnx,have-sampled-metric-cnt:Sampled metric counters enabled in APM.
+- xlnx,num-of-counters: Number of counters in APM
+- xlnx,metric-count-width: Metric Counter width (32/64)
+- xlnx,metrics-sample-count-width: Sampled metric counter width
+- xlnx,global-count-width: Global Clock counter width
+- clocks: Input clock specifier.
+
+Optional properties:
+- xlnx,id-filter-32bit: APM is in 32-bit mode
+
+Example:
+++++++++
+
+apm: apm@44a00000 {
+ compatible = "xlnx,axi-perf-monitor";
+ interrupt-parent = <&axi_intc_1>;
+ interrupts = <1 2>;
+ reg = <0x44a00000 0x1000>;
+ clocks = <&clkc 15>;
+ xlnx,enable-profile = <0>;
+ xlnx,enable-trace = <0>;
+ xlnx,num-monitor-slots = <4>;
+ xlnx,enable-event-count = <1>;
+ xlnx,enable-event-log = <1>;
+ xlnx,have-sampled-metric-cnt = <1>;
+ xlnx,num-of-counters = <8>;
+ xlnx,metric-count-width = <32>;
+ xlnx,metrics-sample-count-width = <32>;
+ xlnx,global-count-width = <32>;
+ xlnx,metric-count-scale = <1>;
+ xlnx,id-filter-32bit;
+};
--
2.1.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver
2019-12-09 14:23 [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver shubhrajyoti.datta
2019-12-09 14:23 ` [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM shubhrajyoti.datta
@ 2019-12-09 15:30 ` Greg KH
1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2019-12-09 15:30 UTC (permalink / raw)
To: shubhrajyoti.datta
Cc: linux-kernel, devicetree, michal.simek, robh+dt,
Shubhrajyoti Datta, Subbaraya Sundeep Bhatta
On Mon, Dec 09, 2019 at 07:53:24PM +0530, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>
> Added driver for Xilinx AXI performance monitor IP.
>
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2:
> Updated the header
>
> drivers/uio/Kconfig | 12 ++
> drivers/uio/Makefile | 1 +
> drivers/uio/uio_xilinx_apm.c | 358 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 371 insertions(+)
> create mode 100644 drivers/uio/uio_xilinx_apm.c
>
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index 202ee81..de30312 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -165,4 +165,16 @@ config UIO_HV_GENERIC
> to network and storage devices from userspace.
>
> If you compile this as a module, it will be called uio_hv_generic.
> +
> +config UIO_XILINX_APM
> + tristate "Xilinx AXI Performance Monitor driver"
> + depends on MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP
> + help
> + This driver is developed for AXI Performance Monitor IP, designed to
> + monitor AXI4 traffic for performance analysis of AXI bus in the
> + system. Driver maps HW registers and parameters to userspace.
> +
> + To compile this driver as a module, choose M here; the module
> + will be called uio_xilinx_apm.
> +
> endif
> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> index c285dd2..b3464d8 100644
> --- a/drivers/uio/Makefile
> +++ b/drivers/uio/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
> obj-$(CONFIG_UIO_MF624) += uio_mf624.o
> obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
> obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
> +obj-$(CONFIG_UIO_XILINX_APM) += uio_xilinx_apm.o
> diff --git a/drivers/uio/uio_xilinx_apm.c b/drivers/uio/uio_xilinx_apm.c
> new file mode 100644
> index 0000000..3f69922
> --- /dev/null
> +++ b/drivers/uio/uio_xilinx_apm.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx AXI Performance Monitor
> + *
> + * Description:
> + * This driver is developed for AXI Performance Monitor IP,
> + * designed to monitor AXI4 traffic for performance analysis
> + * of AXI bus in the system. Driver maps HW registers and parameters
> + * to userspace. Userspace need not clear the interrupt of IP since
> + * driver clears the interrupt.
> + *
> + * Copyright (c) 2019 Xilinx Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/uio_driver.h>
> +
> +#define XAPM_IS_OFFSET 0x0038 /* Interrupt Status Register */
> +#define DRV_NAME "xilinxapm_uio"
> +#define DRV_VERSION "1.0"
No need for a version, the code is in the kernel tree, use the version
of the kernel instead please.
> +#define UIO_DUMMY_MEMSIZE 4096
> +#define XAPM_MODE_ADVANCED 1
> +#define XAPM_MODE_PROFILE 2
> +#define XAPM_MODE_TRACE 3
> +
> +/**
> + * struct xapm_param - HW parameters structure
> + * @mode: Mode in which APM is working
> + * @maxslots: Maximum number of Slots in APM
> + * @eventcnt: Event counting enabled in APM
> + * @eventlog: Event logging enabled in APM
> + * @sampledcnt: Sampled metric counters enabled in APM
> + * @numcounters: Number of counters in APM
> + * @metricwidth: Metric Counter width (32/64)
> + * @sampledwidth: Sampled metric counter width
> + * @globalcntwidth: Global Clock counter width
> + * @scalefactor: Scaling factor
> + * @isr: Interrupts info shared to userspace
> + * @is_32bit_filter: Flags for 32bit filter
> + * @clk: Clock handle
> + */
> +struct xapm_param {
> + u32 mode;
> + u32 maxslots;
> + u32 eventcnt;
> + u32 eventlog;
> + u32 sampledcnt;
> + u32 numcounters;
> + u32 metricwidth;
> + u32 sampledwidth;
> + u32 globalcntwidth;
> + u32 scalefactor;
> + u32 isr;
> + bool is_32bit_filter;
> + struct clk *clk;
You seem to copy this structure to hardware? How? This is not a
definition of a proper hardware interface. Or are you copying it to
userspace? If so, even worse! This is not how to properly define such
a structure at all.
> +};
> +
> +/**
> + * struct xapm_dev - Global driver structure
> + * @info: uio_info structure
> + * @param: xapm_param structure
> + * @regs: IOmapped base address
> + */
> +struct xapm_dev {
> + struct uio_info info;
> + struct xapm_param param;
> + void __iomem *regs;
> +};
> +
> +/**
> + * xapm_handler - Interrupt handler for APM
> + * @irq: IRQ number
> + * @info: Pointer to uio_info structure
> + *
> + * Return: Always returns IRQ_HANDLED
> + */
> +static irqreturn_t xapm_handler(int irq, struct uio_info *info)
> +{
> + struct xapm_dev *xapm = (struct xapm_dev *)info->priv;
DO you need to cast?
> + void *ptr;
> +
> + ptr = (unsigned long *)xapm->info.mem[1].addr;
No need to cast. And ptr is a void, not unsigned long *.
> + /* Clear the interrupt and copy the ISR value to userspace */
> + xapm->param.isr = readl(xapm->regs + XAPM_IS_OFFSET);
> + writel(xapm->param.isr, xapm->regs + XAPM_IS_OFFSET);
> + memcpy(ptr, &xapm->param, sizeof(struct xapm_param));
Where did you just copy this memory to?
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * xapm_getprop - Retrieves dts properties to param structure
> + * @pdev: Pointer to platform device
> + * @param: Pointer to param structure
> + *
> + * Returns: '0' on success and failure value on error
> + */
> +static int xapm_getprop(struct platform_device *pdev, struct xapm_param *param)
> +{
> + u32 mode = 0;
> + int ret;
> + struct device_node *node;
> +
> + node = pdev->dev.of_node;
> +
> + /* Retrieve required dts properties and fill param structure */
> + ret = of_property_read_u32(node, "xlnx,enable-profile", &mode);
> + if (ret < 0)
> + dev_info(&pdev->dev, "no property xlnx,enable-profile\n");
If it's an error, make it dev_err(). No need to be noisy otherwise.
Same for all of these.
> + else if (mode)
> + param->mode = XAPM_MODE_PROFILE;
> +
> + ret = of_property_read_u32(node, "xlnx,enable-trace", &mode);
> + if (ret < 0)
> + dev_info(&pdev->dev, "no property xlnx,enable-trace\n");
> + else if (mode)
> + param->mode = XAPM_MODE_TRACE;
> +
> + ret = of_property_read_u32(node, "xlnx,num-monitor-slots",
> + ¶m->maxslots);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,num-monitor-slots");
Doesn't of_property_read_u32() print an error if it can not be found?
Don't duplicate the message please. Same for all of these.
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,enable-event-count",
> + ¶m->eventcnt);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,enable-event-count");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,enable-event-log",
> + ¶m->eventlog);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,enable-event-log");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,have-sampled-metric-cnt",
> + ¶m->sampledcnt);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,have-sampled-metric-cnt");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,num-of-counters",
> + ¶m->numcounters);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,num-of-counters");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,metric-count-width",
> + ¶m->metricwidth);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,metric-count-width");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,metrics-sample-count-width",
> + ¶m->sampledwidth);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property metrics-sample-count-width");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,global-count-width",
> + ¶m->globalcntwidth);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,global-count-width");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(node, "xlnx,metric-count-scale",
> + ¶m->scalefactor);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no property xlnx,metric-count-scale");
> + return ret;
> + }
> +
> + param->is_32bit_filter = of_property_read_bool(node,
> + "xlnx,id-filter-32bit");
> +
> + return 0;
> +}
> +
> +/**
> + * xapm_probe - Driver probe function
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Returns: '0' on success and failure value on error
> + */
> +
Why the blank line? Why the kernel doc for static functions?
> +static int xapm_probe(struct platform_device *pdev)
> +{
> + struct xapm_dev *xapm;
> + struct resource *res;
> + int irq;
> + int ret;
> + void *ptr;
> +
> + xapm = devm_kzalloc(&pdev->dev, (sizeof(struct xapm_dev)), GFP_KERNEL);
> + if (!xapm)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + xapm->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(xapm->regs)) {
> + dev_err(&pdev->dev, "unable to iomap registers\n");
> + return PTR_ERR(xapm->regs);
> + }
> +
> + xapm->param.clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(xapm->param.clk)) {
> + if (PTR_ERR(xapm->param.clk) != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "axi clock error\n");
> + return PTR_ERR(xapm->param.clk);
> + }
> +
> + ret = clk_prepare_enable(xapm->param.clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to enable clock.\n");
> + return ret;
> + }
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> + /* Initialize mode as Advanced so that if no mode in dts, default
> + * is Advanced
> + */
Odd commenting style, this isn't the network stack :)
> + xapm->param.mode = XAPM_MODE_ADVANCED;
> + ret = xapm_getprop(pdev, &xapm->param);
> + if (ret < 0)
> + goto err_clk_dis;
> +
> + xapm->info.mem[0].name = "xilinx_apm";
Not driver name? Why have that #define then?
> + xapm->info.mem[0].addr = res->start;
> + xapm->info.mem[0].size = resource_size(res);
> + xapm->info.mem[0].memtype = UIO_MEM_PHYS;
> +
> + xapm->info.mem[1].addr = (unsigned long)kzalloc(UIO_DUMMY_MEMSIZE,
> + GFP_KERNEL);
> + ptr = (unsigned long *)xapm->info.mem[1].addr;
> + xapm->info.mem[1].size = UIO_DUMMY_MEMSIZE;
> + xapm->info.mem[1].memtype = UIO_MEM_LOGICAL;
> +
> + xapm->info.name = "axi-pmon";
> + xapm->info.version = DRV_VERSION;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "unable to get irq\n");
> + ret = irq;
> + goto err_clk_dis;
> + }
> +
> + xapm->info.irq = irq;
> + xapm->info.handler = xapm_handler;
> + xapm->info.priv = xapm;
> + xapm->info.irq_flags = IRQF_SHARED;
> +
> + memcpy(ptr, &xapm->param, sizeof(struct xapm_param));
Where did you just copy this to?
> +
> + ret = uio_register_device(&pdev->dev, &xapm->info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "unable to register to UIO\n");
> + goto err_clk_dis;
> + }
> +
> + platform_set_drvdata(pdev, xapm);
> +
> + dev_info(&pdev->dev, "Probed Xilinx APM\n");
Again, this should be quiet if all goes well, didn't I say that before?
Finally, why do you need a UIO driver at all here? Why can't the
current ones work properly for you?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM
2019-12-09 14:23 ` [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM shubhrajyoti.datta
@ 2019-12-10 11:53 ` Michal Simek
2019-12-19 18:43 ` Rob Herring
1 sibling, 0 replies; 5+ messages in thread
From: Michal Simek @ 2019-12-10 11:53 UTC (permalink / raw)
To: shubhrajyoti.datta, linux-kernel, devicetree
Cc: michal.simek, gregkh, robh+dt, Shubhrajyoti Datta
On 09. 12. 19 15:23, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>
> Add the devicetree binding for xilinx APM.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2:
> patch added
>
> .../devicetree/bindings/perf/xilinx_apm.txt | 44 ++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/perf/xilinx_apm.txt
>
> diff --git a/Documentation/devicetree/bindings/perf/xilinx_apm.txt b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
> new file mode 100644
> index 0000000..a11c82e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
> @@ -0,0 +1,44 @@
> +* Xilinx AXI Performance monitor IP
> +
> +Required properties:
> +- compatible: "xlnx,axi-perf-monitor"
> +- interrupts: Should contain APM interrupts.
> +- interrupt-parent: Must be core interrupt controller.
> +- reg: Should contain APM registers location and length.
> +- xlnx,enable-profile: Enables the profile mode.
> +- xlnx,enable-trace: Enables trace mode.
> +- xlnx,num-monitor-slots: Maximum number of slots in APM.
> +- xlnx,enable-event-count: Enable event count.
> +- xlnx,enable-event-log: Enable event logging.
> +- xlnx,have-sampled-metric-cnt:Sampled metric counters enabled in APM.
> +- xlnx,num-of-counters: Number of counters in APM
> +- xlnx,metric-count-width: Metric Counter width (32/64)
> +- xlnx,metrics-sample-count-width: Sampled metric counter width
> +- xlnx,global-count-width: Global Clock counter width
> +- clocks: Input clock specifier.
> +
> +Optional properties:
> +- xlnx,id-filter-32bit: APM is in 32-bit mode
> +
> +Example:
> +++++++++
> +
> +apm: apm@44a00000 {
> + compatible = "xlnx,axi-perf-monitor";
> + interrupt-parent = <&axi_intc_1>;
> + interrupts = <1 2>;
> + reg = <0x44a00000 0x1000>;
> + clocks = <&clkc 15>;
> + xlnx,enable-profile = <0>;
> + xlnx,enable-trace = <0>;
> + xlnx,num-monitor-slots = <4>;
> + xlnx,enable-event-count = <1>;
> + xlnx,enable-event-log = <1>;
> + xlnx,have-sampled-metric-cnt = <1>;
> + xlnx,num-of-counters = <8>;
> + xlnx,metric-count-width = <32>;
> + xlnx,metrics-sample-count-width = <32>;
> + xlnx,global-count-width = <32>;
> + xlnx,metric-count-scale = <1>;
> + xlnx,id-filter-32bit;
> +};
>
If you look at all latest bindings everybody are ask to convert them to
yaml. That's why please do it too.
Thanks,
Michal
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM
2019-12-09 14:23 ` [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM shubhrajyoti.datta
2019-12-10 11:53 ` Michal Simek
@ 2019-12-19 18:43 ` Rob Herring
1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2019-12-19 18:43 UTC (permalink / raw)
To: shubhrajyoti.datta
Cc: linux-kernel, devicetree, michal.simek, gregkh, Shubhrajyoti Datta
On Mon, Dec 09, 2019 at 07:53:25PM +0530, shubhrajyoti.datta@gmail.com wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
>
> Add the devicetree binding for xilinx APM.
>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2:
> patch added
>
> .../devicetree/bindings/perf/xilinx_apm.txt | 44 ++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/perf/xilinx_apm.txt
As Michal said, DT schema please.
>
> diff --git a/Documentation/devicetree/bindings/perf/xilinx_apm.txt b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
> new file mode 100644
> index 0000000..a11c82e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/xilinx_apm.txt
> @@ -0,0 +1,44 @@
> +* Xilinx AXI Performance monitor IP
> +
> +Required properties:
> +- compatible: "xlnx,axi-perf-monitor"
Version?
> +- interrupts: Should contain APM interrupts.
> +- interrupt-parent: Must be core interrupt controller.
Drop this.
> +- reg: Should contain APM registers location and length.
> +- xlnx,enable-profile: Enables the profile mode.
> +- xlnx,enable-trace: Enables trace mode.
> +- xlnx,num-monitor-slots: Maximum number of slots in APM.
> +- xlnx,enable-event-count: Enable event count.
> +- xlnx,enable-event-log: Enable event logging.
> +- xlnx,have-sampled-metric-cnt:Sampled metric counters enabled in APM.
> +- xlnx,num-of-counters: Number of counters in APM
> +- xlnx,metric-count-width: Metric Counter width (32/64)
> +- xlnx,metrics-sample-count-width: Sampled metric counter width
> +- xlnx,global-count-width: Global Clock counter width
All these synthesis time config and not discoverable?
Tell h/w designers to add feature registers so all this is
discoverable. </rant>
> +- clocks: Input clock specifier.
> +
> +Optional properties:
> +- xlnx,id-filter-32bit: APM is in 32-bit mode
> +
> +Example:
> +++++++++
> +
> +apm: apm@44a00000 {
> + compatible = "xlnx,axi-perf-monitor";
> + interrupt-parent = <&axi_intc_1>;
> + interrupts = <1 2>;
> + reg = <0x44a00000 0x1000>;
> + clocks = <&clkc 15>;
> + xlnx,enable-profile = <0>;
> + xlnx,enable-trace = <0>;
> + xlnx,num-monitor-slots = <4>;
> + xlnx,enable-event-count = <1>;
> + xlnx,enable-event-log = <1>;
> + xlnx,have-sampled-metric-cnt = <1>;
> + xlnx,num-of-counters = <8>;
> + xlnx,metric-count-width = <32>;
> + xlnx,metrics-sample-count-width = <32>;
> + xlnx,global-count-width = <32>;
> + xlnx,metric-count-scale = <1>;
> + xlnx,id-filter-32bit;
> +};
> --
> 2.1.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-19 19:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 14:23 [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver shubhrajyoti.datta
2019-12-09 14:23 ` [PATCH v2 2/2] devicetree: bindings: Add the binding doc for xilinx APM shubhrajyoti.datta
2019-12-10 11:53 ` Michal Simek
2019-12-19 18:43 ` Rob Herring
2019-12-09 15:30 ` [PATCH v2 1/2] uio: uio_xilinx_apm: Add Xilinx AXI performance monitor driver Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).