[dev-4.7 patch 04/11] pinctrl (aspeed) introduce Aspeed SoC jtag driver
Andrew Jeffery
andrew at aj.id.au
Mon Aug 22 13:01:23 AEST 2016
Hi Vadim,
On Wed, 2016-08-10 at 07:03 +0000, vadimp at mellanox.com wrote:
> From: Vadim Pasternak <vadimp at mellanox.com>
>
> The Kconfig controlling compilation of this code is:
> drivers/pinctrl/aspeed/Kconfig:config PINCTRL_ASPEED_JTAG
I can't see what the JTAG driver has to do with the pinctrl subsystem.
Can you clarify? Perhaps the driver should be moved elsewhere?
>
> Porting peci driver from 3.18 to 4.7.
>
> Signed-off-by: Vadim Pasternak <vadimp at mellanox.com>
> ---
> drivers/pinctrl/aspeed/Kconfig | 6 +
> drivers/pinctrl/aspeed/Makefile | 4 +
> drivers/pinctrl/aspeed/aspeed-jtag.c | 919 +++++++++++++++++++++++++++++++++++
> 3 files changed, 929 insertions(+)
> create mode 100644 drivers/pinctrl/aspeed/aspeed-jtag.c
>
> diff --git a/drivers/pinctrl/aspeed/Kconfig b/drivers/pinctrl/aspeed/Kconfig
> index ee45a96..0d289ab 100644
> --- a/drivers/pinctrl/aspeed/Kconfig
> +++ b/drivers/pinctrl/aspeed/Kconfig
> @@ -22,3 +22,9 @@ config PINCTRL_ASPEED_G5
> help
> Say Y here to enable pin controller support for Aspeed's 5th
> generation AST SoCs. GPIO is provided by a separate GPIO driver.
> +
> +config PINCTRL_ASPEED_JTAG
> + tristate "ASPEED JTAG control"
> + default n
> + help
> + Driver for ASPEED JTAG controller driver
> diff --git a/drivers/pinctrl/aspeed/Makefile b/drivers/pinctrl/aspeed/Makefile
> index 0f4b876..e229fd1 100644
> --- a/drivers/pinctrl/aspeed/Makefile
> +++ b/drivers/pinctrl/aspeed/Makefile
> @@ -1,5 +1,9 @@
> # Aspeed pinctrl support
>
> +ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := \
> + -I$(srctree)/arch/arm/mach-aspeed/include
> +
> obj-$(CONFIG_PINCTRL_ASPEED) += pinctrl-aspeed.o
> obj-$(CONFIG_PINCTRL_ASPEED_G4) += pinctrl-aspeed-g4.o
> obj-$(CONFIG_PINCTRL_ASPEED_G5) += pinctrl-aspeed-g5.o
> +obj-$(CONFIG_PINCTRL_ASPEED_JTAG) += aspeed-jtag.o
> diff --git a/drivers/pinctrl/aspeed/aspeed-jtag.c b/drivers/pinctrl/aspeed/aspeed-jtag.c
> new file mode 100644
> index 0000000..11ac953
> --- /dev/null
> +++ b/drivers/pinctrl/aspeed/aspeed-jtag.c
> @@ -0,0 +1,919 @@
> +/*
> + * driver/pincntrl/aspeed_jtag.c
> + *
> + * ASPEED JTAG controller driver
> + *
> + * Copyright (C) 2012-2020 ASPEED Technology Inc.
> + *
> + * 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.
> + *
> + * History:
> + * 2012.08.06: Initial version [Ryan Chen]
> + * 2016.0806: Porting to kernel 4.7
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define AST_JTAG_DATA 0x00
> +#define AST_JTAG_INST 0x04
> +#define AST_JTAG_CTRL 0x08
> +#define AST_JTAG_ISR 0x0C
> +#define AST_JTAG_SW 0x10
> +#define AST_JTAG_TCK 0x14
> +#define AST_JTAG_IDLE 0x18
Interestingly the AST2500 datasheet doesn't list the IDLE reg in its
summary list, but does in the register tables. Par for the course I
guess.
> +
> +/* AST_JTAG_CTRL - 0x08 : Engine Control */
> +#define JTAG_ENG_EN (0x1 << 31)
> +#define JTAG_ENG_OUT_EN (0x1 << 30)
> +#define JTAG_FORCE_TMS (0x1 << 29)
> +#define JTAG_IR_UPDATE (0x1 << 26)
> +#define JTAG_INST_LEN_MASK (0x3f << 20)
> +#define JTAG_SET_INST_LEN(x) (x << 20)
> +#define JTAG_SET_INST_MSB (0x1 << 19)
> +#define JTAG_TERMINATE_INST (0x1 << 18)
> +#define JTAG_LAST_INST (0x1 << 17)
> +#define JTAG_INST_EN (0x1 << 16)
> +#define JTAG_DATA_LEN_MASK (0x3f << 4)
> +#define JTAG_DR_UPDATE (0x1 << 10)
JTAG_DR_UPDATE is out of order?
> +#define JTAG_DATA_LEN(x) (x << 4)
> +#define JTAG_SET_DATA_MSB (0x1 << 3)
> +#define JTAG_TERMINATE_DATA (0x1 << 2)
> +#define JTAG_LAST_DATA (0x1 << 1)
> +#define JTAG_DATA_EN (0x1)
Minor issue with formatting: Sometimes indentation of #define values is
used to indicate available values within a bitfield, but it seems like
a number of these defines are dealing with independent bits yet have
varying indentations. Is the list meant to be formatted this way? The
defines below also look odd.
> +
> +/* AST_JTAG_ISR - 0x0C : INterrupt status and enable */
> +#define JTAG_INST_PAUSE (0x1 << 19)
> +#define JTAG_INST_COMPLETE (0x1 << 18)
> +#define JTAG_DATA_PAUSE (0x1 << 17)
> +#define JTAG_DATA_COMPLETE (0x1 << 16)
> +
> +#define JTAG_INST_PAUSE_EN (0x1 << 3)
> +#define JTAG_INST_COMPLETE_EN (0x1 << 2)
> +#define JTAG_DATA_PAUSE_EN (0x1 << 1)
> +#define JTAG_DATA_COMPLETE_EN (0x1)
> +
> +/* AST_JTAG_SW - 0x10 : Software Mode and Status */
> +#define JTAG_SW_MODE_EN (0x1 << 19)
> +#define JTAG_SW_MODE_TCK (0x1 << 18)
> +#define JTAG_SW_MODE_TMS (0x1 << 17)
> +#define JTAG_SW_MODE_TDIO (0x1 << 16)
> +#define JTAG_STS_INST_PAUSE (0x1 << 2)
> +#define JTAG_STS_DATA_PAUSE (0x1 << 1)
> +#define JTAG_STS_ENG_IDLE (0x1)
> +
> +/* AST_JTAG_TCK - 0x14 : TCK Control */
> +#define JTAG_TCK_INVERSE (0x1 << 31)
> +#define JTAG_TCK_DIVISOR_MASK (0x7ff)
> +#define JTAG_GET_TCK_DIVISOR(x) (x & 0x7ff)
> +
> +/* AST_JTAG_IDLE - 0x18 : Ctroller set for go to IDLE */
> +#define JTAG_GO_IDLE (0x1)
> +
> +enum aspeed_jtag_xfer_mode {
> + JTAG_XFER_HW_MODE = 0,
> + JTAG_XFER_SW_MODE = 1,
> +};
> +
> +struct runtest_idle {
> + enum aspeed_jtag_xfer_mode mode; /* 0 :HW mode, 1: SW mode */
> + unsigned char reset; /* Test Logic Reset */
> + unsigned char end; /* o: idle, 1: ir pause, 2: drpause */
> + unsigned char tck; /* keep tck */
> +};
> +
> +struct sir_xfer {
> + enum aspeed_jtag_xfer_mode mode; /* 0 :HW mode, 1: SW mode */
> + unsigned short length; /* bits */
> + unsigned int tdi;
> + unsigned int tdo;
> + unsigned char endir; /* 0: idle, 1:pause */
> +};
> +
> +struct sdr_xfer {
> + enum aspeed_jtag_xfer_mode mode; /* 0 :HW mode, 1: SW mode */
> + unsigned char direct; /* 0 ; read , 1 : write */
> + unsigned short length; /* bits */
> + unsigned int *tdio;
> + unsigned char enddr; /* 0: idle, 1:pause */
> +};
> +
> +#define JTAGIOC_BASE 'T'
> +
> +#define AST_JTAG_IOCRUNTEST _IOW(JTAGIOC_BASE, 0, struct runtest_idle)
> +#define AST_JTAG_IOCSIR _IOWR(JTAGIOC_BASE, 1, struct sir_xfer)
> +#define AST_JTAG_IOCSDR _IOWR(JTAGIOC_BASE, 2, struct sdr_xfer)
> +#define AST_JTAG_SIOCFREQ _IOW(JTAGIOC_BASE, 3, unsigned int)
> +#define AST_JTAG_GIOCFREQ _IOR(JTAGIOC_BASE, 4, unsigned int)
> +
> +struct aspeed_jtag_info {
> + void __iomem *reg_base;
> + struct device *dev;
> + u8 sts; /* 0: idle, 1:irpause 2:drpause */
> + int irq; /* JTAG IRQ number */
> + u32 flag;
> + wait_queue_head_t jtag_wq;
> + bool is_open;
> +};
> +
> +struct aspeed_jtag_info *aspeed_jtag;
> +static DEFINE_SPINLOCK(jtag_state_lock);
> +
> +static inline u32
> +aspeed_jtag_read(struct aspeed_jtag_info *aspeed_jtag, u32 reg)
> +{
> + return readl(aspeed_jtag->reg_base + reg);
> +}
> +
> +static inline void
> +aspeed_jtag_write(struct aspeed_jtag_info *aspeed_jtag, u32 val, u32 reg)
> +{
> + dev_dbg(aspeed_jtag->dev, "reg = 0x%08x, val = 0x%08x\n", reg, val);
> + writel(val, aspeed_jtag->reg_base + reg);
> +}
> +
> +u32 ast_get_pclk(void)
> +{
> + return 0;
Should this be interacting with the clk infrastructure?
> +}
> +
> +void
> +aspeed_jtag_set_freq(struct aspeed_jtag_info *aspeed_jtag, unsigned int freq)
> +{
> + u16 i;
> +
> + for (i = 0; i < 0x7ff; i++) {
What is 0x7ff? Having dissected the code a little more it seems it's
the maximum divisor value...
> + if ((ast_get_pclk() / (i + 1)) <= freq)
By the ast_get_pclk() implementation above this is trivially true.
Anyway, this could be re-written to avoid the loop. Roughly from the
datasheet:
Ptck = Ppclk * (Dtck + 1)
Ptck / Ppclk = Dtck + 1
(Ptck / Ppclk) - 1 = Dtck
This does assume that 0 < Ppclk <= Ptck, but a broader case is handled
by the code as it stands (i.e. it's robust in the face of the 0
returned by ast_get_pclk()).
Assuming both ast_get_pclk() gives Hz and not a period, the code might
look like:
if (0 < ast_get_pclk() && freq < ast_get_pclk()) {
i = (ast_get_pclk() / freq) - 1;
} else {
WARN(freq > ast_get_pclk(), ...);
WARN(freq == 0, ...);
i = 0;
}
> + break;
> + }
> + aspeed_jtag_write(aspeed_jtag,
> + ((aspeed_jtag_read(aspeed_jtag, AST_JTAG_TCK) &
> + ~JTAG_TCK_DIVISOR_MASK) | i),
> + AST_JTAG_TCK);
> +}
> +
> +unsigned int aspeed_jtag_get_freq(struct aspeed_jtag_info *aspeed_jtag)
> +{
> + return ast_get_pclk() /
> + (JTAG_GET_TCK_DIVISOR(aspeed_jtag_read(aspeed_jtag,
> + AST_JTAG_TCK)) + 1);
Again, always 0 due to ast_get_pclk() implementation
> +}
> +
> +void dummy(struct aspeed_jtag_info *aspeed_jtag, unsigned int cnt)
> +{
> + int i = 0;
> +
> + for (i = 0; i < cnt; i++)
> + aspeed_jtag_read(aspeed_jtag, AST_JTAG_SW);
> +}
> +
> +static u8 TCK_Cycle(struct aspeed_jtag_info *aspeed_jtag, u8 TMS, u8 TDI)
> +{
> + u8 tdo;
> +
> + /* TCK = 0 */
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN |
> + (TMS * JTAG_SW_MODE_TMS) | (TDI * JTAG_SW_MODE_TDIO),
> + AST_JTAG_SW);
> +
> + dummy(aspeed_jtag, 10);
> +
> + /* TCK = 1 */
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN | JTAG_SW_MODE_TCK |
> + (TMS * JTAG_SW_MODE_TMS) | (TDI * JTAG_SW_MODE_TDIO),
> + AST_JTAG_SW);
> +
> + if (aspeed_jtag_read(aspeed_jtag, AST_JTAG_SW) & JTAG_SW_MODE_TDIO)
> + tdo = 1;
> + else
> + tdo = 0;
> +
> + dummy(aspeed_jtag, 10);
> +
> + /* TCK = 0 */
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN | (TMS *
> + JTAG_SW_MODE_TMS) | (TDI * JTAG_SW_MODE_TDIO),
> + AST_JTAG_SW);
> +
> + return tdo;
> +}
> +
> +void aspeed_jtag_wait_instruction_complete(struct aspeed_jtag_info *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, (aspeed_jtag->flag ==
> + JTAG_INST_COMPLETE));
> + dev_err(aspeed_jtag->dev, "\n");
> + aspeed_jtag->flag = 0;
> +}
> +
> +void aspeed_jtag_wait_data_pause_complete(struct aspeed_jtag_info *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, (aspeed_jtag->flag ==
> + JTAG_DATA_PAUSE));
> + dev_err(aspeed_jtag->dev, "\n");
> + aspeed_jtag->flag = 0;
> +}
> +
> +void aspeed_jtag_wait_data_complete(struct aspeed_jtag_info *aspeed_jtag)
> +{
> + wait_event_interruptible(aspeed_jtag->jtag_wq, (aspeed_jtag->flag ==
> + JTAG_DATA_COMPLETE));
> + dev_err(aspeed_jtag->dev, "\n");
> + aspeed_jtag->flag = 0;
> +}
> +
> +/* JTAG_reset() is to generate at least 9 TMS high and 1 TMS low to force
> + * devices into Run-Test/Idle State.
> + */
> +void aspeed_jtag_run_test_idle(struct aspeed_jtag_info *aspeed_jtag,
> + struct runtest_idle *runtest)
> +{
> + int i = 0;
> +
> + dev_dbg(aspeed_jtag->dev, ":%s mode\n", runtest->mode ? "SW":"HW");
> +
> + if (runtest->mode) {
> + /* SW mode from idle , from pause, -- > to pause, to idle */
> + if (runtest->reset) {
> + for (i = 0; i < 10; i++)
> + TCK_Cycle(aspeed_jtag, 1, 0);
> + }
> +
> + switch (aspeed_jtag->sts) {
> + case 0:
> + if (runtest->end == 1) {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRSCan */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRSCan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IRCap */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRExit1 */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IRPause */
> + aspeed_jtag->sts = 1;
> + } else if (runtest->end == 2) {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRSCan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRCap */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRExit1 */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRPause */
> + aspeed_jtag->sts = 1;
> + } else {
> + TCK_Cycle(aspeed_jtag, 0, 0);/* IDLE */
> + aspeed_jtag->sts = 0;
> + }
> + break;
> +
> + case 1:
> + /* from IR/DR Pause */
> + if (runtest->end == 1) {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Exit2 IR/DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Updt IR/DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRSCan */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRSCan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IRCap */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRExit1 */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IRPause */
> + aspeed_jtag->sts = 1;
> + } else if (runtest->end == 2) {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Exit2 IR/DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Update IR/DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRSCan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRCap */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRExit1 */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRPause */
> + aspeed_jtag->sts = 1;
> + } else {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Exit2 IR/DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Updt IR/DR */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IDLE */
> + aspeed_jtag->sts = 0;
> + }
> + break;
> +
> + default:
> + dev_err(aspeed_jtag->dev,
> + "aspeed_jtag_run_test_idle error\n");
> + break;
> + }
> +
> + /* stay on IDLE for at least TCK cycle */
> + for (i = 0; i < runtest->tck; i++)
> + TCK_Cycle(aspeed_jtag, 0, 0);
> + } else {
> + /* dis sw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, AST_JTAG_SW);
> + mdelay(1);
> + /* x TMS high + 1 TMS low */
> + if (runtest->reset)
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN |
> + JTAG_ENG_OUT_EN | JTAG_FORCE_TMS,
> + AST_JTAG_CTRL);
> + else
> + aspeed_jtag_write(aspeed_jtag, JTAG_GO_IDLE,
> + AST_JTAG_IDLE);
> + mdelay(1);
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN |
> + JTAG_SW_MODE_TDIO, AST_JTAG_SW);
> + aspeed_jtag->sts = 0;
> + }
> +}
> +
> +int aspeed_jtag_sir_xfer(struct aspeed_jtag_info *aspeed_jtag,
> + struct sir_xfer *sir)
> +{
> + int i = 0;
> +
> + dev_err(aspeed_jtag->dev, "%s mode, ENDIR : %d, len : %d\n",
> + sir->mode ? "SW":"HW", sir->endir, sir->length);
> +
> + if (sir->mode) {
> + if (aspeed_jtag->sts) {
> + /* from IR/DR Pause */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Exit2 IR / DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Update IR /DR */
> + }
> +
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRSCan */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRSCan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* CapIR */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* ShiftIR */
> +
> + sir->tdo = 0;
> + for (i = 0; i < sir->length; i++) {
> + if (i == (sir->length - 1)) {
Would be better to terminate the loop early and then do the body
unconditionally afterwards.
> + /* IRExit1 */
> + sir->tdo |= TCK_Cycle(aspeed_jtag, 1,
> + sir->tdi & 0x1);
> + } else {
> + /* ShiftIR */
> + sir->tdo |= TCK_Cycle(aspeed_jtag, 0,
> + sir->tdi & 0x1);
> + sir->tdi >>= 1;
> + sir->tdo <<= 1;
> + }
> + }
> +
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IRPause */
> +
> + /* stop pause */
> + if (sir->endir == 0) {
> + /* go to idle */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRExit2 */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* IRUpdate */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IDLE */
> + }
> + } else {
> + /* hw mode - disable sw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, AST_JTAG_SW);
> + aspeed_jtag_write(aspeed_jtag, sir->tdi, AST_JTAG_INST);
> +
> + if (sir->endir) {
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN |
> + JTAG_ENG_OUT_EN |
> + JTAG_SET_INST_LEN(sir->length),
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN |
> + JTAG_ENG_OUT_EN |
> + JTAG_SET_INST_LEN(sir->length) |
> + JTAG_INST_EN, AST_JTAG_CTRL);
> + } else {
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN |
> + JTAG_ENG_OUT_EN | JTAG_LAST_INST |
> + JTAG_SET_INST_LEN(sir->length),
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN |
> + JTAG_ENG_OUT_EN | JTAG_LAST_INST |
> + JTAG_SET_INST_LEN(sir->length) |
> + JTAG_INST_EN, AST_JTAG_CTRL);
> + }
> +
> + aspeed_jtag_wait_instruction_complete(aspeed_jtag);
> + sir->tdo = aspeed_jtag_read(aspeed_jtag, AST_JTAG_INST);
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN |
> + JTAG_SW_MODE_TDIO, AST_JTAG_SW);
> + }
> +
> + aspeed_jtag->sts = sir->endir;
> +
> + return 0;
> +}
> +
> +int aspeed_jtag_sdr_xfer(struct aspeed_jtag_info *aspeed_jtag,
> + struct sdr_xfer *sdr)
> +{
> + unsigned int index = 0;
> + u32 shift_bits = 0;
> + u32 tdo = 0;
> + u32 remain_xfer = sdr->length;
> +
> + dev_err(aspeed_jtag->dev, "%s mode, ENDDR : %d, len : %d\n",
> + sdr->mode ? "SW":"HW", sdr->enddr, sdr->length);
> +
> + if (sdr->mode) {
> + /* SW mode */
> + if (aspeed_jtag->sts) {
> + /* from IR/DR Pause */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Exit2 IR / DR */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* Update IR /DR */
> + }
> +
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRScan */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRCap */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRShift */
> +
> + while (remain_xfer) {
> + if (sdr->direct) {
> + /* write */
> + if ((shift_bits % 32) == 0)
> + dev_err(aspeed_jtag->dev, "W dr->dr_data[%d]: %x\n",
> + index, sdr->tdio[index]);
I guess we always read before write? Or is this not actually an error?
> +
> + tdo = (sdr->tdio[index] >> (shift_bits % 32)) &
> + 0x1;
> + dev_err(aspeed_jtag->dev, "%d\n", tdo);
Should this be dev_err()?
> + if (remain_xfer == 1)
> + /* DRExit1 */
> + TCK_Cycle(aspeed_jtag, 1, tdo);
> + else
> + /* DRShit */
> + TCK_Cycle(aspeed_jtag, 0, tdo);
> + } else {
> + /* read */
> + if (remain_xfer == 1)
> + /* DRExit1 */
> + tdo = TCK_Cycle(aspeed_jtag, 1, tdo);
> + else
> + /* DRShit */
> + tdo = TCK_Cycle(aspeed_jtag, 0, tdo);
> +
> + dev_err(aspeed_jtag->dev, "%d\n", tdo);
Should this be dev_err()?
> + sdr->tdio[index] |= (tdo << (shift_bits % 32));
> +
> + if ((shift_bits % 32) == 0)
> + dev_err(aspeed_jtag->dev, "R dr->dr_data[%d]: %x\n",
> + index, sdr->tdio[index]);
> + }
> + shift_bits++;
> + remain_xfer--;
> + if ((shift_bits % 32) == 0)
> + index++;
> + }
> +
> + TCK_Cycle(aspeed_jtag, 0, 0); /* DRPause */
> +
> + if (sdr->enddr == 0) {
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRExit2 */
> + TCK_Cycle(aspeed_jtag, 1, 0); /* DRUpdate */
> + TCK_Cycle(aspeed_jtag, 0, 0); /* IDLE */
> + }
> + } else {
> + /* hw mode */
> + aspeed_jtag_write(aspeed_jtag, 0, AST_JTAG_SW);
> + while (remain_xfer) {
> + if (sdr->direct) {
> + dev_err(aspeed_jtag->dev,
> + "W dr->dr_data[%d]: %x\n", index,
> + sdr->tdio[index]);
Again, is this meant to be an error?
> + aspeed_jtag_write(aspeed_jtag,
> + sdr->tdio[index],
> + AST_JTAG_DATA);
> + } else {
> + aspeed_jtag_write(aspeed_jtag, 0,
> + AST_JTAG_DATA);
> + }
> +
> + if (remain_xfer > 32) {
> + shift_bits = 32;
> + /* read bytes were not equals to column length
> + * => Pause-DR
> + */
> + dev_err(aspeed_jtag->dev, "shit bits %d\n",
dev_err() is appropriate for shit bits, but are these actually shit
bits? Or shift bits? I won't bother commenting for all the other
dev_err() instances, please check all instances though.
> + shift_bits);
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_DATA_LEN(shift_bits),
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_DATA_LEN(shift_bits) |
> + JTAG_DATA_EN,
> + AST_JTAG_CTRL);
> + aspeed_jtag_wait_data_pause_complete(
> + aspeed_jtag);
> + } else {
> + /* read bytes equals to column length =>
> + * Update-DR
> + */
> + shift_bits = remain_xfer;
> + dev_err(aspeed_jtag->dev,
> + "shit bits %d with last\n",
> + shift_bits);
> + if (sdr->enddr) {
> + dev_err(aspeed_jtag->dev, "DR Keep Pause\n");
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_DR_UPDATE |
> + JTAG_DATA_LEN(shift_bits),
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_DR_UPDATE |
> + JTAG_DATA_LEN(shift_bits) |
> + JTAG_DATA_EN, AST_JTAG_CTRL);
> + } else {
> + dev_err(aspeed_jtag->dev, "DR go IDLE\n");
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_LAST_DATA |
> + JTAG_DATA_LEN(shift_bits),
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag,
> + JTAG_ENG_EN | JTAG_ENG_OUT_EN |
> + JTAG_LAST_DATA |
> + JTAG_DATA_LEN(shift_bits) |
> + JTAG_DATA_EN, AST_JTAG_CTRL);
> + }
> + aspeed_jtag_wait_data_complete(aspeed_jtag);
> + }
> +
> + if (!sdr->direct) {
> + if (shift_bits < 32)
> + sdr->tdio[index] =
> + aspeed_jtag_read(aspeed_jtag,
> + AST_JTAG_DATA) >>
> + (32 - shift_bits);
> + else
> + sdr->tdio[index] =
> + aspeed_jtag_read(aspeed_jtag,
> + AST_JTAG_DATA);
> + dev_err(aspeed_jtag->dev,
> + "R dr->dr_data[%d]: %x\n",
> + index, sdr->tdio[index]);
> + }
> +
> + remain_xfer = remain_xfer - shift_bits;
> + index++;
> + dev_err(aspeed_jtag->dev, "remain_xfer %d\n",
> + remain_xfer);
> + }
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN |
> + JTAG_SW_MODE_TDIO, AST_JTAG_SW);
> + }
> +
> + aspeed_jtag->sts = sdr->enddr;
> +
> + return 0;
> +}
> +
> +static irqreturn_t aspeed_jtag_interrupt(int this_irq, void *dev_id)
> +{
> + u32 status;
> + struct aspeed_jtag_info *aspeed_jtag = dev_id;
> +
> + status = aspeed_jtag_read(aspeed_jtag, AST_JTAG_ISR);
> + dev_err(aspeed_jtag->dev, "sts %x\n", status);
> +
> + if (status & JTAG_INST_COMPLETE) {
> + aspeed_jtag_write(aspeed_jtag, JTAG_INST_COMPLETE |
> + (status & 0xf), AST_JTAG_ISR);
> + aspeed_jtag->flag = JTAG_INST_COMPLETE;
> + }
> +
> + if (status & JTAG_DATA_PAUSE) {
> + aspeed_jtag_write(aspeed_jtag, JTAG_DATA_PAUSE |
> + (status & 0xf), AST_JTAG_ISR);
> + aspeed_jtag->flag = JTAG_DATA_PAUSE;
> + }
> +
> + if (status & JTAG_DATA_COMPLETE) {
> + aspeed_jtag_write(aspeed_jtag, JTAG_DATA_COMPLETE |
> + (status & 0xf), AST_JTAG_ISR);
> + aspeed_jtag->flag = JTAG_DATA_COMPLETE;
> + }
> +
> + if (aspeed_jtag->flag) {
> + wake_up_interruptible(&aspeed_jtag->jtag_wq);
> + return IRQ_HANDLED;
> + } else {
> + dev_err(aspeed_jtag->dev, "aspeed_jtag irq status (%x)\n",
> + status);
> + return IRQ_NONE;
> + }
> +}
> +
> +static long jtag_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct aspeed_jtag_info *aspeed_jtag = file->private_data;
> + void __user *argp = (void __user *)arg;
> + struct sir_xfer sir;
> + struct sdr_xfer sdr;
> + struct runtest_idle run_idle;
> + int ret = 0;
> +
> + switch (cmd) {
> + case AST_JTAG_GIOCFREQ:
> + ret = __put_user(aspeed_jtag_get_freq(aspeed_jtag),
> + (unsigned int __user *)arg);
> + break;
> +
> + case AST_JTAG_SIOCFREQ:
> + if ((unsigned int )arg > ast_get_pclk())
> + ret = -EFAULT;
> + else
> + aspeed_jtag_set_freq(aspeed_jtag, (unsigned int)arg);
> + break;
> +
> + case AST_JTAG_IOCRUNTEST:
> + if (copy_from_user(&run_idle, argp,
> + sizeof(struct runtest_idle)))
> + ret = -EFAULT;
> + else
> + aspeed_jtag_run_test_idle(aspeed_jtag, &run_idle);
> +
> + break;
> +
> + case AST_JTAG_IOCSIR:
> + if (copy_from_user(&sir, argp, sizeof(struct sir_xfer)))
> + ret = -EFAULT;
> + else
> + aspeed_jtag_sir_xfer(aspeed_jtag, &sir);
> +
> + if (copy_to_user(argp, &sir, sizeof(struct sdr_xfer)))
> + ret = -EFAULT;
> +
> + break;
> +
> + case AST_JTAG_IOCSDR:
> + if (copy_from_user(&sdr, argp, sizeof(struct sdr_xfer)))
> + ret = -EFAULT;
> + else
> + aspeed_jtag_sdr_xfer(aspeed_jtag, &sdr);
> +
> + if (copy_to_user(argp, &sdr, sizeof(struct sdr_xfer)))
> + ret = -EFAULT;
> +
> + break;
> + default:
> +
> + return -ENOTTY;
> + }
> +
> + return ret;
> +}
> +
> +static int jtag_open(struct inode *inode, struct file *file)
> +{
> + spin_lock(&jtag_state_lock);
> + if (aspeed_jtag->is_open) {
> + spin_unlock(&jtag_state_lock);
> + return -EBUSY;
> + }
> +
> + aspeed_jtag->is_open = true;
> + file->private_data = aspeed_jtag;
> + spin_unlock(&jtag_state_lock);
> +
> + return 0;
> +}
> +
> +static int jtag_release(struct inode *inode, struct file *file)
> +{
> + struct aspeed_jtag_info *drvdata = file->private_data;
> +
> + spin_lock(&jtag_state_lock);
> + drvdata->is_open = false;
> + spin_unlock(&jtag_state_lock);
> +
> + return 0;
> +}
> +
> +static ssize_t show_sts(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_jtag_info *aspeed_jtag = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", aspeed_jtag->sts ? "Pause" : "Idle");
> +}
> +
> +static DEVICE_ATTR(sts, S_IRUGO, show_sts, NULL);
> +
> +static ssize_t show_frequency(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct aspeed_jtag_info *aspeed_jtag = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "Frequency (%d)\n", ast_get_pclk() /
> + (JTAG_GET_TCK_DIVISOR(aspeed_jtag_read(aspeed_jtag,
> + AST_JTAG_TCK)) + 1));
> +}
> +
> +static ssize_t store_frequency(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + unsigned long input_val;
> + struct aspeed_jtag_info *aspeed_jtag = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &input_val);
> + if (ret)
> + return ret;
> +
> + aspeed_jtag_set_freq(aspeed_jtag, input_val);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(freq, S_IRUGO | S_IWUSR, show_frequency, store_frequency);
> +
> +static struct attribute *jtag_sysfs_entries[] = {
> + &dev_attr_freq.attr,
> + &dev_attr_sts.attr,
> + NULL
> +};
> +
> +static struct attribute_group jtag_attribute_group = {
> + .attrs = jtag_sysfs_entries,
> +};
> +
> +static const struct file_operations aspeed_jtag_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = jtag_ioctl,
> + .open = jtag_open,
> + .release = jtag_release,
> +};
> +
> +struct miscdevice aspeed_jtag_misc = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = "aspeed-jtag",
> + .fops = &aspeed_jtag_fops,
> +};
> +
> +static void aspeed_jtag_remove_sysfs_group(void *_dev)
> +{
> + struct device *dev = _dev;
> +
> + sysfs_remove_group(&dev->kobj, &jtag_attribute_group);
> +}
> +
> +
> +static int aspeed_jtag_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + int ret = 0;
> +
> + if (!of_device_is_compatible(pdev->dev.of_node,
> + "aspeed,aspeed2500-jtag"))
> + return -ENODEV;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "cannot get IORESOURCE_MEM\n");
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + aspeed_jtag = devm_kzalloc(&pdev->dev, sizeof(*aspeed_jtag),
> + GFP_KERNEL);
> + if (!aspeed_jtag)
> + return -ENOMEM;
> +
> + aspeed_jtag->reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (!aspeed_jtag->reg_base) {
> + ret = -EIO;
> + goto out_region;
> + }
> +
> + aspeed_jtag->irq = platform_get_irq(pdev, 0);
> + if (aspeed_jtag->irq < 0) {
> + dev_err(&pdev->dev, "no irq specified\n");
> + ret = -ENOENT;
> + goto out_region;
> + }
> +
> + /* Init JTAG */
> + aspeed_toggle_scu_reset(SCU_RESET_JTAG, 3);
> +
> + /* Eanble Clock */
> + aspeed_jtag_write(aspeed_jtag, JTAG_ENG_EN | JTAG_ENG_OUT_EN,
> + AST_JTAG_CTRL);
> + aspeed_jtag_write(aspeed_jtag, JTAG_SW_MODE_EN | JTAG_SW_MODE_TDIO,
> + AST_JTAG_SW);
> +
> + ret = devm_request_irq(&pdev->dev, aspeed_jtag->irq,
> + aspeed_jtag_interrupt, 0,
> + "aspeed-jtag", aspeed_jtag);
> + if (ret) {
> + dev_info(&pdev->dev, "aspeed_jtag Unable to get IRQ");
> + goto out_region;
> + }
> +
> + aspeed_jtag_write(aspeed_jtag, JTAG_INST_PAUSE | JTAG_INST_COMPLETE |
> + JTAG_DATA_PAUSE | JTAG_DATA_COMPLETE |
> + JTAG_INST_PAUSE_EN | JTAG_INST_COMPLETE_EN |
> + JTAG_DATA_PAUSE_EN | JTAG_DATA_COMPLETE_EN,
> + AST_JTAG_ISR);
> +
> + aspeed_jtag->flag = 0;
> + init_waitqueue_head(&aspeed_jtag->jtag_wq);
> +
> + ret = misc_register(&aspeed_jtag_misc);
> + if (ret) {
> + dev_err(&pdev->dev, "aspeed_jtag : failed to register misc\n");
> + goto out_region;
> + }
> +
> + aspeed_jtag->dev = &pdev->dev;
> + platform_set_drvdata(pdev, aspeed_jtag);
> + dev_set_drvdata(aspeed_jtag_misc.this_device, aspeed_jtag);
> +
> + ret = sysfs_create_group(&pdev->dev.kobj, &jtag_attribute_group);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "aspeed_jtag : failed to create sysfs attributes.\n");
> + return -1;
> + }
> + ret = devm_add_action(&pdev->dev, aspeed_jtag_remove_sysfs_group,
> + &pdev->dev);
> + if (ret) {
> + aspeed_jtag_remove_sysfs_group(&pdev->dev);
> + dev_err(&pdev->dev,
> + "Failed to add sysfs cleanup action (%d)\n",
> + ret);
> + return ret;
> + }
> +
> + dev_info(&pdev->dev, "aspeed_jtag : driver successfully loaded.\n");
> +
> + return 0;
It might help readability to split out some of the initialisation into
helper functions?
Cheers,
Andrew
> +
> +out_region:
> + release_mem_region(res->start, res->end - res->start + 1);
> +out:
> + dev_warn(&pdev->dev, "aspeed_jtag : driver init failed ret (%d)\n", ret);
> + return ret;
> +}
> +
> +static int aspeed_jtag_remove(struct platform_device *pdev)
> +{
> + misc_deregister(&aspeed_jtag_misc);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int
> +aspeed_jtag_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + return 0;
> +}
> +
> +static int
> +aspeed_jtag_resume(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +
> +#else
> +#define aspeed_jtag_suspend NULL
> +#define aspeed_jtag_resume NULL
> +#endif
> +
> +static const struct of_device_id aspeed_jtag_of_table[] = {
> + { .compatible = "aspeed,aspeed2500-jtag", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_jtag_of_table);
> +
> +static struct platform_driver aspeed_jtag_driver = {
> + .probe = aspeed_jtag_probe,
> + .remove = aspeed_jtag_remove,
> + .suspend = aspeed_jtag_suspend,
> + .resume = aspeed_jtag_resume,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .owner = THIS_MODULE,
> + .of_match_table = aspeed_jtag_of_table,
> + },
> +};
> +
> +module_platform_driver(aspeed_jtag_driver);
> +
> +MODULE_AUTHOR("Ryan Chen <ryan_chen at aspeedtech.com>");
> +MODULE_DESCRIPTION("AST JTAG LIB Driver");
> +MODULE_LICENSE("GPL");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160822/9a8a5fd7/attachment-0001.sig>
More information about the openbmc
mailing list