Hi Wjatscheslaw,
The driver looks mostly ok, some minor stuff below. Please run
scripts/checkpatch.pl, there are some coding style issues.
How do you use this driver? From C code, or is there some command
planned?
On Tue, Jul 24, 2012 at 03:48:24PM +0200, Wjatscheslaw Stoljarski (Slawa) wrote:
> Signed-off-by: Wjatscheslaw Stoljarski
> <wjatscheslaw.stoljar...@kiwigrid.com>
> ---
> drivers/Kconfig | 1 +
> drivers/Makefile | 1 +
> drivers/misc/Kconfig | 27 ++++
> drivers/misc/Makefile | 5 +
> drivers/misc/jtag.c | 395
> +++++++++++++++++++++++++++++++++++++++++++++++++
linewrap here, please use git send-email.
> +#define MAX_DEVICES 16
> +
> +static LIST_HEAD(jtag_device_list);
Each device is added to this list and removed from this list on remove,
but otherwise it's unused. Please remove.
> +
> +struct jtag_info {
> + struct jtag_platdata *pdata;
> + struct cdev cdev;
> + unsigned int devices; /* Number of devices found in the jtag chain */
> + struct list_head device_entry;
> + /* Instruction register length of every device in the chain */
> + unsigned int ir_len[]; /* [devices] */
Better use a fixed len array here: ir_len[MAX_DEVICES]
It makes the code more obviously correct.
> +};
> +
> +static const unsigned long bypass = 0xFFFFFFFF;
> +
> +static void pulseTMS0(const struct jtag_platdata *pdata)
Better name it to pulse_tms0. Lower case function names are preferred.
> +{
> + gpio_set_value(pdata->pin_tms, 0);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> +}
> +
> +static void pulseTMS1(const struct jtag_platdata *pdata)
> +{
> + gpio_set_value(pdata->pin_tms, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> +}
> +
> +static void jtag_reset(const struct jtag_platdata *pdata)
> +{
> + gpio_set_value(pdata->pin_tms, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> +}
> +
> +static void jtag_output(const struct jtag_platdata *pdata,
> + const unsigned long *data, unsigned int bitlen, int notlast)
> +{
> + unsigned int a;
> + unsigned long mask;
> + gpio_set_value(pdata->pin_tms, 0);
> + while (bitlen > 0) {
> + for (a = *data++, mask = 0x00000001; mask != 0 && bitlen > 0;
> + mask <<= 1, bitlen--) {
> + gpio_set_value(pdata->pin_tdo, (a & mask) ? 1 : 0);
> + if ((bitlen == 1) && !notlast)
> + gpio_set_value(pdata->pin_tms, 1);
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + }
> + }
> +}
> +
> +static int jtag_ioctl(struct cdev *inode, int cmd, void *arg)
> +{
> + int ret = 0;
> + struct jtag_info *info = (struct jtag_info *)inode->priv;
> + int devices = info->devices;
> + struct jtag_cmd *jcmd = (struct jtag_cmd *)arg;
> + struct jtag_platdata *pdata = info->pdata;
> +
> + if (_IOC_TYPE(cmd) != JTAG_IOC_MAGIC) return -ENOTTY;
> + if (_IOC_NR(cmd) > JTAG_IOC_MAXNR) return -ENOTTY;
> +
> + switch (cmd) {
> +
> + case JTAG_GET_DEVICES:
> + /* Returns how many devices found in the chain */
> + ret = info->devices;
> + break;
> +
> + case JTAG_GET_ID:
> + /* Returns ID register of selected device */
> + if ((((struct jtag_rd_id *)arg)->device < 0) ||
> + (((struct jtag_rd_id *)arg)->device >= devices)) {
> + ret = -EINVAL;
> + break;
> + }
> + jtag_reset(pdata);
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + while (devices-- > 0) {
> + unsigned long id = 0;
> + pulseTMS0(pdata);
> + if (gpio_get_value(pdata->pin_tdi)) {
> + unsigned long mask;
> + for (id = 1, mask = 0x00000002; (mask != 0);
> + mask <<= 1) {
> + pulseTMS0(pdata);
> + if (gpio_get_value(pdata->pin_tdi))
> + id |= mask;
> + }
> + }
> + if (devices == ((struct jtag_rd_id *)arg)->device) {
> + ((struct jtag_rd_id *)arg)->id = id;
> + ret = 0;
> + break;
> + }
> + }
> + jtag_reset(pdata);
> + break;
> +
> + case JTAG_SET_IR_LENGTH:
> + /* Sets up IR length of one device */
> + if ((jcmd->device >= 0) && (jcmd->device < devices))
> + info->ir_len[jcmd->device] = jcmd->bitlen;
> + else
> + ret = -EINVAL;
> + break;
> +
> + case JTAG_RESET:
> + /* Resets all JTAG states */
> + jtag_reset(pdata);
> + break;
> +
> + case JTAG_IR_WR:
> + /* Writes Instruction Register
> + If device == -1 writes same Instruction Register in all devices
> + If device >= 0 writes Instruction Register in selected device
> + and loads BYPASS instruction in all others */
> + if ((jcmd->device < -1) || (jcmd->device >= devices)) {
> + ret = -EINVAL;
> + break;
> + }
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + while (devices-- > 0) {
> + if ((jcmd->device == -1) || (jcmd->device == devices))
> + /* Loads desired instruction */
> + jtag_output(pdata, jcmd->data,
> + info->ir_len[devices], devices);
> + else
> + /* Loads BYPASS instruction */
> + jtag_output(pdata, &bypass,
> + info->ir_len[devices], devices);
> + }
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + break;
> +
> + case JTAG_DR_WR:
> + /* Writes Data Register of all devices
> + If device == -1 writes same Data Register in all devices
> + If device >= 0 writes Data Register in selected device
> + and loads BYPASS instruction in all others */
/*
* multiline comments
* like this please
*/
> + if ((jcmd->device < -1) || (jcmd->device >= devices)) {
> + ret = -EINVAL;
> + break;
> + }
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + while (devices-- > 0) {
> + if ((jcmd->device == -1) || (devices == jcmd->device))
> + /* Loads desired data */
> + jtag_output(pdata, jcmd->data, jcmd->bitlen,
> + devices);
> + else
> + /* Loads 1 dummy bit in BYPASS data register */
> + jtag_output(pdata, &bypass, 1, devices);
> + }
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + break;
> +
> + case JTAG_DR_RD:
> + /* Reads data register of selected device */
> + if ((jcmd->device < 0) || (jcmd->device >= devices))
> + ret = -EINVAL;
> + else {
> + unsigned long mask;
> + int bitlen = jcmd->bitlen;
> + unsigned long *data = jcmd->data;
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + devices -= (jcmd->device + 1);
> + while (devices-- > 0)
> + pulseTMS0(pdata);
> + while (bitlen > 0) {
> + for (*data = 0, mask = 0x00000001;
> + (mask != 0) && (bitlen > 0);
> + mask <<= 1, bitlen--) {
> + if (bitlen == 1)
> + pulseTMS1(pdata);
> + else
> + pulseTMS0(pdata);
> + if (gpio_get_value(pdata->pin_tdi))
> + *data |= mask;
> + }
> + data++;
> + }
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + }
> + break;
> +
> + case JTAG_CLK:
> + /* Generates arg clock pulses */
> + gpio_set_value(pdata->pin_tms, 0);
> + while ((*(unsigned int *) arg)--) {
> + gpio_set_value(pdata->pin_tclk, 0);
> + gpio_set_value(pdata->pin_tclk, 1);
> + }
> + break;
> +
> + default:
> + ret = -EFAULT;
> + }
> +
> + return ret;
> +}
> +
> +struct file_operations jtag_operations = {
> + .ioctl = jtag_ioctl,
> +};
> +
> +static int jtag_probe(struct device_d *pdev)
> +{
> + int i, ret;
> + struct jtag_info *info;
> + struct jtag_platdata *pdata = pdev->platform_data;
> +
> + /* Setup gpio pins */
> + gpio_direction_output(pdata->pin_tms, 0);
> + gpio_direction_output(pdata->pin_tclk, 1);
> + gpio_direction_output(pdata->pin_tdo, 0);
> + gpio_direction_input(pdata->pin_tdi);
> + if (pdata->use_pin_trst) {
> + /* Keep fixed at 1 because some devices in the chain could
> + not use it, to reset chain use jtag_reset() */
> + gpio_direction_output(pdata->pin_trst, 1);
> + }
> +
> + /* Find how many devices in chain */
> + jtag_reset(pdata);
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + gpio_set_value(pdata->pin_tdo, 1);
> + /* Fills all IR with bypass instruction */
> + for (i = 0; i < 32 * MAX_DEVICES; i++)
> + pulseTMS0(pdata);
> + pulseTMS1(pdata);
> + pulseTMS1(pdata);
> + pulseTMS1(pdata);
> + pulseTMS0(pdata);
> + pulseTMS0(pdata);
> + gpio_set_value(pdata->pin_tdo, 0);
> + /* Fills all 1-bit bypass register with 0 */
> + for (i = 0; i < MAX_DEVICES + 2; i++)
> + pulseTMS0(pdata);
> + gpio_set_value(pdata->pin_tdo, 1);
> + /* Counts chain's bit length */
> + for (i = 0; i < MAX_DEVICES + 1; i++) {
> + pulseTMS0(pdata);
> + if (gpio_get_value(pdata->pin_tdi))
> + break;
> + }
> + dev_notice(pdev, "%d devices found in chain\n", i);
> +
> + /* Allocate structure with chain specific infos */
> + info = xzalloc(sizeof(struct jtag_info) + sizeof(info->ir_len[0]) * i);
> + if (!info) {
> + dev_err(pdev, "out of kernel memory\n");
> + return -ENOMEM;
> + }
the x* memory allocation functions never return an error, barebox panics
instead. They are suitable for small memory allocations like above. You
don't have to check the result.
> + info->devices = i;
> + INIT_LIST_HEAD(&info->device_entry);
> + list_add(&info->device_entry, &jtag_device_list);
> + info->pdata = pdata;
> + pdev->priv = info;
> +
> + info->cdev.name = JTAG_NAME;
> + info->cdev.dev = pdev;
> + info->cdev.ops = &jtag_operations;
> + info->cdev.priv = info;
> + ret = devfs_create(&info->cdev);
> +
> + if (ret)
> + goto fail_devfs_create;
> +
> + return 0;
> +
> +fail_devfs_create:
> + pdev->priv = NULL;
> + free(info);
> + return ret;
> +}
> +
> +static void jtag_info(struct device_d *pdev)
> +{
> + int dn, ret;
> + struct jtag_rd_id jid;
> + struct jtag_info *info = pdev->priv;
> +
> + printf(" JTAG:\n");
> + printf(" Devices found: %d\n", info->devices);
> + for(dn = 0; dn < info->devices; dn++) {
> + jid.device = dn;
> + ret = jtag_ioctl(&info->cdev, JTAG_GET_ID, &jid);
> + printf(" Device number: %d\n", dn);
> + if ( ret == -1 )
> + printf(" JTAG_GET_ID failed: %s\n", strerror(errno));
> + else
> + printf(" ID: 0x%lX\n", jid.id);
> + }
> +}
> +
> +static void jtag_remove(struct device_d *pdev)
> +{
> + struct jtag_info *info = (struct jtag_info *) pdev->priv;
> +
> + list_del(&info->device_entry);
> + devfs_remove(&info->cdev);
> + pdev->priv = NULL;
> + free(info);
> + dev_notice(pdev, "Device removed\n");
> +}
> +
> +static struct driver_d jtag_driver = {
> + .name = JTAG_NAME,
> + .probe = jtag_probe,
> + .remove = jtag_remove,
> + .info = jtag_info,
> +};
> +
> +static int jtag_module_init(void)
> +{
> + return register_driver(&jtag_driver);
> +}
> +
> +device_initcall(jtag_module_init);
> +
> +MODULE_AUTHOR("Davide Rizzo <elpa.ri...@gmail.com>");
> +MODULE_AUTHOR("Wjatscheslaw Stoljarski
> <wjatscheslaw.stoljar...@kiwigrid.com>");
> +MODULE_DESCRIPTION("JTAG bitbang driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/jtag.h b/include/jtag.h
> new file mode 100644
> index 0000000..d8f0606
> --- /dev/null
> +++ b/include/jtag.h
> @@ -0,0 +1,113 @@
> +/*
> + * include/linux/jtag.h
> + *
> + * Written Aug 2009 by Davide Rizzo <elpa.ri...@gmail.com>
> + * Ported to barebox Jul 2012 by
> + * Wjatscheslaw Stoljarski <wjatscheslaw.stoljar...@kiwigrid.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +/*
> + * This driver manages one or more jtag chains controlled by host pins.
> + * Jtag chains must be defined during setup using jtag_platdata structs.
> + * All operations must be done from user programs using ioctls to /dev/jtag
> + * Typical operation sequence is:
> + * - open() the device (normally /dev/jtag)
> + * - ioctl JTAG_GET_DEVICES reads how many devices in the chain
> + * (repeat for each chip in the chain)
> + * - ioctl JTAG_GET_ID identifies the chip
> + * - ioctl JTAG_SET_IR_LENGTH sets the instruction register length
> + * Before accessing the data registers, instruction registers' lenghtes
> + * MUST be programmed for all chips.
> + * After this initialization, you can execute JTAG_IR_WR,
> JTAG_DR_RD, JTAG_DR_WR
> + * commands in any sequence.
> + */
> +
> +#ifndef __JTAG_H__
> +#define __JTAG_H__
> +
> +#ifdef __KERNEL__
You don't need this ifdef
> +/* Controller's pin_tdi must be connected to last device's pin_tdo */
> +/* Controller's pin_tdo must be connected to first device's pin_tdi */
> +struct jtag_platdata {
> + unsigned int pin_tclk;
> + unsigned int pin_tms;
> + unsigned int pin_tdi;
> + unsigned int pin_tdo;
> + unsigned int pin_trst;
Better gpio_t*? It makes it more obvious that these variables refer to
gpios.
> + int use_pin_trst;
> +};
> +#endif /* __KERNEL__ */
> +
> +#define JTAG_NAME "jtag"
> +
> +/* structures used for passing arguments to ioctl */
> +
> +struct jtag_rd_id {
> + int device; /* Device in the chain */
> + unsigned long id;
> +};
> +
> +struct jtag_cmd {
> + int device; /* Device in the chain (-1 = all devices) */
> + unsigned int bitlen; /* Bit length of the register to be transfered */
> + unsigned long *data; /* Data to be transfered */
> +};
> +
> +/* Use 'j' as magic number */
> +#define JTAG_IOC_MAGIC 'j'
> +
> +/* ioctl commands */
> +
> +/* Resets jtag chain status, arg is ignored */
> +#define JTAG_RESET _IO(JTAG_IOC_MAGIC, 0)
> +
> +/* Returns the number of devices in the jtag chain, arg is ignored. */
> +#define JTAG_GET_DEVICES _IO(JTAG_IOC_MAGIC, 1)
> +
> +/* arg must point to a jtag_rd_id structure.
> + Fills up the id field with ID of selected device */
> +#define JTAG_GET_ID _IOR(JTAG_IOC_MAGIC, 2, struct jtag_rd_id)
> +
> +/* arg must point to a struct jtag_cmd.
> + Programs the Instruction Register length of specified device at
> bitlen value.
> + *data is ignored. */
> +#define JTAG_SET_IR_LENGTH _IOW(JTAG_IOC_MAGIC, 3, struct jtag_rd_id)
> +
> +/* arg must point to a struct jtag_cmd.
> + Writes *data in the Instruction Register of selected device, and BYPASS
> + instruction into Instruction Registers of all other devices in
> the chain.
> + If device == -1, the Instruction Registers of all devices are programmed
> + to the same value.
> + bitlen is always ignored, before using this command you have to
> program all
> + Instruction Register's lengthes with JTAG_SET_IR_LENGTH command. */
> +#define JTAG_IR_WR _IOW(JTAG_IOC_MAGIC, 4, struct jtag_cmd)
> +
> +/* arg must point to a struct jtag_cmd.
> + Reads data register of selected device, with length bitlen */
> +#define JTAG_DR_RD _IOR(JTAG_IOC_MAGIC, 5, struct jtag_cmd)
> +
> +/* arg must point to a struct jtag_cmd.
> + Writes data register of selected device, with length bitlen.
> + If device == -1, writes same data on all devices. */
> +#define JTAG_DR_WR _IOW(JTAG_IOC_MAGIC, 6, struct jtag_cmd)
> +
> +/* Generates arg pulses on TCLK pin */
> +#define JTAG_CLK _IOW(JTAG_IOC_MAGIC, 7, unsigned int)
> +
> +#define JTAG_IOC_MAXNR 9
> +
> +#endif /* __JTAG_H__ */
> --
> 1.7.9.5
>
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
