diff mbox

[V2,04/15] coresight: tmc: introducing new header file

Message ID 1460483692-25061-5-git-send-email-mathieu.poirier@linaro.org
State Superseded
Headers show

Commit Message

Mathieu Poirier April 12, 2016, 5:54 p.m. UTC
The amount of #define, enumeration and structure definition
is big enough to justify moving them to a new header file.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

---
 drivers/hwtracing/coresight/coresight-tmc.c | 102 +----------------------
 drivers/hwtracing/coresight/coresight-tmc.h | 122 ++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 101 deletions(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-tmc.h

-- 
2.5.0

Comments

Mathieu Poirier April 15, 2016, 4:03 p.m. UTC | #1
On 14 April 2016 at 11:33, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 12/04/16 18:54, Mathieu Poirier wrote:

>>

>> The amount of #define, enumeration and structure definition

>> is big enough to justify moving them to a new header file.

>>

>

>

>> +/* TMC_STS - 0x00C */

>> +#define TMC_STS_TRIGGERED      BIT(1)

>

>

> ...

>

>> +#define TMC_AXICTL_WR_BURST_LEN 0xF00

>

>

> Nit: The value above signifies, 16 data transfers per burst.

> So ideally it would be good to rename it to reflect that. say,

>

> TMC_AXICTL_WR_BURST_16


Will do.  But I'll have to do this in a separate patch then the
grouping of STS_ and FFCR_ defines you're referring to below since it
will also require changes to the .c files.

>

>

>

>> +/* TMC_FFCR - 0x304 */

>> +#define TMC_FFCR_EN_FMT                BIT(0)

>> +#define TMC_FFCR_EN_TI         BIT(1)

>> +#define TMC_FFCR_FON_FLIN      BIT(4)

>> +#define TMC_FFCR_FON_TRIG_EVT  BIT(5)

>> +#define TMC_FFCR_FLUSHMAN      BIT(6)

>> +#define TMC_FFCR_TRIGON_TRIGIN BIT(8)

>> +#define TMC_FFCR_STOP_ON_FLUSH BIT(12)

>> +

>> +#define TMC_STS_TMCREADY_BIT   2

>

>

>> +#define TMC_FFCR_FLUSHMAN_BIT  6

>

>

> nit: It would be nice to group the STS_ and FFCR_ bits together.

> Also I see that the defintion for

> TMC_STS_FULL is added in a completely unrelated patch (TMC-ETF AUX SPACE

> patch ?). It would be good to add it either here or in a different patch.


TMC_STS_FULL is not added here because at this point it is not used by
the code - it is only added later when it is useful.  Please get back
to me if I missed your point.

>

> Otherwise looks good.

>

> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

>

> Thanks

> Suzuki
Mathieu Poirier April 15, 2016, 4:15 p.m. UTC | #2
On 15 April 2016 at 10:08, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 15/04/16 17:03, Mathieu Poirier wrote:

>>

>> On 14 April 2016 at 11:33, Suzuki K Poulose <Suzuki.Poulose@arm.com>

>> wrote:

>>>

>>> On 12/04/16 18:54, Mathieu Poirier wrote:

>>>>

>>>>

>>>> The amount of #define, enumeration and structure definition

>>>> is big enough to justify moving them to a new header file.

>>>>

>>>

>>>

>>>> +/* TMC_STS - 0x00C */

>>>> +#define TMC_STS_TRIGGERED      BIT(1)

>>>

>>>

>>>

>>> ...

>>>

>>>> +#define TMC_AXICTL_WR_BURST_LEN 0xF00

>>>

>>>

>>>

>>> Nit: The value above signifies, 16 data transfers per burst.

>>> So ideally it would be good to rename it to reflect that. say,

>>>

>>> TMC_AXICTL_WR_BURST_16

>>

>>

>> Will do.  But I'll have to do this in a separate patch then the

>> grouping of STS_ and FFCR_ defines you're referring to below since it

>> will also require changes to the .c files.

>

>

> Yes, I don't expect this change to be part of the patch. Separate patch

> is fine.

>

>>

>>>

>>>

>>>

>>>> +/* TMC_FFCR - 0x304 */

>>>> +#define TMC_FFCR_EN_FMT                BIT(0)

>>>> +#define TMC_FFCR_EN_TI         BIT(1)

>>>> +#define TMC_FFCR_FON_FLIN      BIT(4)

>>>> +#define TMC_FFCR_FON_TRIG_EVT  BIT(5)

>>>> +#define TMC_FFCR_FLUSHMAN      BIT(6)

>>>> +#define TMC_FFCR_TRIGON_TRIGIN BIT(8)

>>>> +#define TMC_FFCR_STOP_ON_FLUSH BIT(12)

>>>> +

>>>> +#define TMC_STS_TMCREADY_BIT   2

>>>

>>>

>>>

>>>> +#define TMC_FFCR_FLUSHMAN_BIT  6

>>>

>>>

>>>

>>> nit: It would be nice to group the STS_ and FFCR_ bits together.

>>> Also I see that the defintion for

>>> TMC_STS_FULL is added in a completely unrelated patch (TMC-ETF AUX SPACE

>>> patch ?). It would be good to add it either here or in a different patch.

>>

>>

>> TMC_STS_FULL is not added here because at this point it is not used by

>> the code - it is only added later when it is useful.

>

>

> I agree. But the patch which introduces the definition doesn't deal with

> TMC_STS_ at all either.


Patch 13/15 is using the TMC_STS_FULL define on line 147.  Am I
missing your point?

>Thats why I said, either here or in a different

> patch

> than what is there. May be you could club the change above and the STS_FULL

> into a new single patch. Its not mandatory though.

>

> Suzuki
diff mbox

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index d211aeec49f8..01a7ce2974f7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -30,107 +30,7 @@ 
 #include <linux/amba/bus.h>
 
 #include "coresight-priv.h"
-
-#define TMC_RSZ			0x004
-#define TMC_STS			0x00c
-#define TMC_RRD			0x010
-#define TMC_RRP			0x014
-#define TMC_RWP			0x018
-#define TMC_TRG			0x01c
-#define TMC_CTL			0x020
-#define TMC_RWD			0x024
-#define TMC_MODE		0x028
-#define TMC_LBUFLEVEL		0x02c
-#define TMC_CBUFLEVEL		0x030
-#define TMC_BUFWM		0x034
-#define TMC_RRPHI		0x038
-#define TMC_RWPHI		0x03c
-#define TMC_AXICTL		0x110
-#define TMC_DBALO		0x118
-#define TMC_DBAHI		0x11c
-#define TMC_FFSR		0x300
-#define TMC_FFCR		0x304
-#define TMC_PSCR		0x308
-#define TMC_ITMISCOP0		0xee0
-#define TMC_ITTRFLIN		0xee8
-#define TMC_ITATBDATA0		0xeec
-#define TMC_ITATBCTR2		0xef0
-#define TMC_ITATBCTR1		0xef4
-#define TMC_ITATBCTR0		0xef8
-
-/* register description */
-/* TMC_CTL - 0x020 */
-#define TMC_CTL_CAPT_EN		BIT(0)
-/* TMC_STS - 0x00C */
-#define TMC_STS_TRIGGERED	BIT(1)
-/* TMC_AXICTL - 0x110 */
-#define TMC_AXICTL_PROT_CTL_B0	BIT(0)
-#define TMC_AXICTL_PROT_CTL_B1	BIT(1)
-#define TMC_AXICTL_SCT_GAT_MODE	BIT(7)
-#define TMC_AXICTL_WR_BURST_LEN 0xF00
-/* TMC_FFCR - 0x304 */
-#define TMC_FFCR_EN_FMT		BIT(0)
-#define TMC_FFCR_EN_TI		BIT(1)
-#define TMC_FFCR_FON_FLIN	BIT(4)
-#define TMC_FFCR_FON_TRIG_EVT	BIT(5)
-#define TMC_FFCR_FLUSHMAN	BIT(6)
-#define TMC_FFCR_TRIGON_TRIGIN	BIT(8)
-#define TMC_FFCR_STOP_ON_FLUSH	BIT(12)
-
-#define TMC_STS_TMCREADY_BIT	2
-#define TMC_FFCR_FLUSHMAN_BIT	6
-
-enum tmc_config_type {
-	TMC_CONFIG_TYPE_ETB,
-	TMC_CONFIG_TYPE_ETR,
-	TMC_CONFIG_TYPE_ETF,
-};
-
-enum tmc_mode {
-	TMC_MODE_CIRCULAR_BUFFER,
-	TMC_MODE_SOFTWARE_FIFO,
-	TMC_MODE_HARDWARE_FIFO,
-};
-
-enum tmc_mem_intf_width {
-	TMC_MEM_INTF_WIDTH_32BITS	= 0x2,
-	TMC_MEM_INTF_WIDTH_64BITS	= 0x3,
-	TMC_MEM_INTF_WIDTH_128BITS	= 0x4,
-	TMC_MEM_INTF_WIDTH_256BITS	= 0x5,
-};
-
-/**
- * struct tmc_drvdata - specifics associated to an TMC component
- * @base:	memory mapped base address for this component.
- * @dev:	the device entity associated to this component.
- * @csdev:	component vitals needed by the framework.
- * @miscdev:	specifics to handle "/dev/xyz.tmc" entry.
- * @spinlock:	only one at a time pls.
- * @read_count:	manages preparation of buffer for reading.
- * @buf:	area of memory where trace data get sent.
- * @paddr:	DMA start location in RAM.
- * @vaddr:	virtual representation of @paddr.
- * @size:	@buf size.
- * @enable:	this TMC is being used.
- * @config_type: TMC variant, must be of type @tmc_config_type.
- * @trigger_cntr: amount of words to store after a trigger.
- */
-struct tmc_drvdata {
-	void __iomem		*base;
-	struct device		*dev;
-	struct coresight_device	*csdev;
-	struct miscdevice	miscdev;
-	spinlock_t		spinlock;
-	int			read_count;
-	bool			reading;
-	char			*buf;
-	dma_addr_t		paddr;
-	void			*vaddr;
-	u32			size;
-	bool			enable;
-	enum tmc_config_type	config_type;
-	u32			trigger_cntr;
-};
+#include "coresight-tmc.h"
 
 static void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
 {
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
new file mode 100644
index 000000000000..2d7d52747b4e
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -0,0 +1,122 @@ 
+/*
+ * Copyright(C) 2015 Linaro Limited. All rights reserved.
+ * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _CORESIGHT_TMC_H
+#define _CORESIGHT_TMC_H
+
+#define TMC_RSZ			0x004
+#define TMC_STS			0x00c
+#define TMC_RRD			0x010
+#define TMC_RRP			0x014
+#define TMC_RWP			0x018
+#define TMC_TRG			0x01c
+#define TMC_CTL			0x020
+#define TMC_RWD			0x024
+#define TMC_MODE		0x028
+#define TMC_LBUFLEVEL		0x02c
+#define TMC_CBUFLEVEL		0x030
+#define TMC_BUFWM		0x034
+#define TMC_RRPHI		0x038
+#define TMC_RWPHI		0x03c
+#define TMC_AXICTL		0x110
+#define TMC_DBALO		0x118
+#define TMC_DBAHI		0x11c
+#define TMC_FFSR		0x300
+#define TMC_FFCR		0x304
+#define TMC_PSCR		0x308
+#define TMC_ITMISCOP0		0xee0
+#define TMC_ITTRFLIN		0xee8
+#define TMC_ITATBDATA0		0xeec
+#define TMC_ITATBCTR2		0xef0
+#define TMC_ITATBCTR1		0xef4
+#define TMC_ITATBCTR0		0xef8
+
+/* register description */
+/* TMC_CTL - 0x020 */
+#define TMC_CTL_CAPT_EN		BIT(0)
+/* TMC_STS - 0x00C */
+#define TMC_STS_TRIGGERED	BIT(1)
+/* TMC_AXICTL - 0x110 */
+#define TMC_AXICTL_PROT_CTL_B0	BIT(0)
+#define TMC_AXICTL_PROT_CTL_B1	BIT(1)
+#define TMC_AXICTL_SCT_GAT_MODE	BIT(7)
+#define TMC_AXICTL_WR_BURST_LEN 0xF00
+/* TMC_FFCR - 0x304 */
+#define TMC_FFCR_EN_FMT		BIT(0)
+#define TMC_FFCR_EN_TI		BIT(1)
+#define TMC_FFCR_FON_FLIN	BIT(4)
+#define TMC_FFCR_FON_TRIG_EVT	BIT(5)
+#define TMC_FFCR_FLUSHMAN	BIT(6)
+#define TMC_FFCR_TRIGON_TRIGIN	BIT(8)
+#define TMC_FFCR_STOP_ON_FLUSH	BIT(12)
+
+#define TMC_STS_TMCREADY_BIT	2
+#define TMC_FFCR_FLUSHMAN_BIT	6
+
+enum tmc_config_type {
+	TMC_CONFIG_TYPE_ETB,
+	TMC_CONFIG_TYPE_ETR,
+	TMC_CONFIG_TYPE_ETF,
+};
+
+enum tmc_mode {
+	TMC_MODE_CIRCULAR_BUFFER,
+	TMC_MODE_SOFTWARE_FIFO,
+	TMC_MODE_HARDWARE_FIFO,
+};
+
+enum tmc_mem_intf_width {
+	TMC_MEM_INTF_WIDTH_32BITS	= 0x2,
+	TMC_MEM_INTF_WIDTH_64BITS	= 0x3,
+	TMC_MEM_INTF_WIDTH_128BITS	= 0x4,
+	TMC_MEM_INTF_WIDTH_256BITS	= 0x5,
+};
+
+/**
+ * struct tmc_drvdata - specifics associated to an TMC component
+ * @base:	memory mapped base address for this component.
+ * @dev:	the device entity associated to this component.
+ * @csdev:	component vitals needed by the framework.
+ * @miscdev:	specifics to handle "/dev/xyz.tmc" entry.
+ * @spinlock:	only one at a time pls.
+ * @read_count:	manages preparation of buffer for reading.
+ * @buf:	area of memory where trace data get sent.
+ * @paddr:	DMA start location in RAM.
+ * @vaddr:	virtual representation of @paddr.
+ * @size:	@buf size.
+ * @enable:	this TMC is being used.
+ * @config_type: TMC variant, must be of type @tmc_config_type.
+ * @trigger_cntr: amount of words to store after a trigger.
+ */
+struct tmc_drvdata {
+	void __iomem		*base;
+	struct device		*dev;
+	struct coresight_device	*csdev;
+	struct miscdevice	miscdev;
+	spinlock_t		spinlock;
+	int			read_count;
+	bool			reading;
+	char			*buf;
+	dma_addr_t		paddr;
+	void __iomem		*vaddr;
+	u32			size;
+	bool			enable;
+	enum tmc_config_type	config_type;
+	u32			trigger_cntr;
+};
+
+#endif