Do you really need to have the commands.[ch] changes here? If you can
leave that alone, it would be preferred, as it doesn't appear to have
anything to do with this patch.
Also, you should not convert header file #includes in .c files to <> if
the header is in the same directory. Indeed, that is what the ""'s help
indicate to developers: it's a local module file. Read any of the 100
or so commit log messages and you will see this was done explicitly.
In jtag.h, you add an extra line when you remove jtag_add_dr_out(), and
that probably deserves to be removed too.
Other than these nits, there remains the fundamental problem that we are
now duplicating code between the out-of-line version in drivers.c and
the inline version in minidriver/minidriver_imp.h.
Once the nits are fixed, you might as well commit it, as the process to
add multiple interface support will probably tear this apart anyway.
--Z
On Tue, 2009-12-08 at 09:39 +0100, Øyvind Harboe wrote:
> Low latency low CPU processing power systems(embedded)
> will benefit greatly from being able to inline certain
> jtag_add_xxx() fn's. The trick is that this has to be
> done in such a way as to allow implementing an OpenOCD
> API with a shared library(eventually) on a PC hosted
> OpenOCD.
>
> Signed-off-by: Øyvind Harboe <oyvind.har...@zylin.com>
> ---
> .gitignore | 4 ++++
> src/jtag/Makefile.am | 32 ++++++++++++++++++++++++--------
> src/jtag/commands.c | 1 +
> src/jtag/commands.h | 2 --
> src/jtag/core.c | 31 ++-----------------------------
> src/jtag/drivers/driver.c | 31 ++++++++++++++++++++++++++++++-
> src/jtag/drivers/minidriver_imp.h | 11 ++++++++++-
> src/jtag/jtag.h | 12 ++++++------
> src/jtag/minidriver.h | 2 +-
> src/jtag/minidriver/minidriver_imp.h | 19 ++++++++++++++++++-
> src/jtag/minidummy/minidummy.c | 1 +
> 11 files changed, 97 insertions(+), 49 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 114fc05..9928129 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -10,6 +10,10 @@
> *.la
> *.in
>
> +# generated source files
> +src/jtag/minidriver_imp.h
> +src/jtag/jtag_minidriver.h
> +
> # editor files
> *.swp
>
> diff --git a/src/jtag/Makefile.am b/src/jtag/Makefile.am
> index da2eddd..3f132d4 100644
> --- a/src/jtag/Makefile.am
> +++ b/src/jtag/Makefile.am
> @@ -9,33 +9,49 @@ SUBDIRS =
> DRIVERFILES =
> libjtag_la_LIBADD =
>
> -if MINIDRIVER
> +CLEANFILES =
> +
> +BUILT_SOURCES =
> +
> +BUILT_SOURCES += minidriver_imp.h
> +CLEANFILES += minidriver_imp.h
>
> -# for minidriver_imp.h
> -AM_CPPFLAGS += -I$(srcdir)/minidriver
> +if MINIDRIVER
>
> if ZY1000
> DRIVERFILES += zy1000/zy1000.c
> -AM_CPPFLAGS += -I$(srcdir)/zy1000
> +JTAG_MINIDRIVER_DIR = $(srcdir)/zy1000
> endif
> if MINIDRIVER_DUMMY
> DRIVERFILES += minidummy/minidummy.c commands.c
> -AM_CPPFLAGS += -I$(srcdir)/minidummy
> +JTAG_MINIDRIVER_DIR = $(srcdir)/minidummy
> endif
>
> +MINIDRIVER_IMP_DIR = $(srcdir)/minidriver
> +
> +jtag_minidriver.h: $(JTAG_MINIDRIVER_DIR)/jtag_minidriver.h
> + cp $< $@
> +
> +BUILT_SOURCES += jtag_minidriver.h
> +
> +CLEANFILES += jtag_minidriver.h
> +
> else
>
> +MINIDRIVER_IMP_DIR = $(srcdir)/drivers
> DRIVERFILES += commands.c
>
> SUBDIRS += drivers
> libjtag_la_LIBADD += $(top_builddir)/src/jtag/drivers/libocdjtagdrivers.la
>
> -# for minidriver_imp.h
> -AM_CPPFLAGS += -I$(srcdir)/drivers
> -
> endif
> +
> # endif // MINIDRIVER
>
> +minidriver_imp.h: $(MINIDRIVER_IMP_DIR)/minidriver_imp.h
> + cp $< $@
> +
> +
> libjtag_la_SOURCES = \
> core.c \
> interface.c \
> diff --git a/src/jtag/commands.c b/src/jtag/commands.c
> index 4e8ce40..6951f03 100644
> --- a/src/jtag/commands.c
> +++ b/src/jtag/commands.c
> @@ -31,6 +31,7 @@
> #include "config.h"
> #endif
>
> +#include <jtag/jtag.h>
> #include "commands.h"
>
> struct cmd_queue_page {
> diff --git a/src/jtag/commands.h b/src/jtag/commands.h
> index 86ded15..b10b545 100644
> --- a/src/jtag/commands.h
> +++ b/src/jtag/commands.h
> @@ -26,8 +26,6 @@
> #ifndef JTAG_COMMANDS_H
> #define JTAG_COMMANDS_H
>
> -#include <jtag/jtag.h>
> -
> /**
> * The inferred type of a scan_command_s structure, indicating whether
> * the command has the host scan in from the device, the host scan out
> diff --git a/src/jtag/core.c b/src/jtag/core.c
> index 373dd7e..8cc346d 100644
> --- a/src/jtag/core.c
> +++ b/src/jtag/core.c
> @@ -31,9 +31,8 @@
> #include "config.h"
> #endif
>
> -#include "jtag.h"
> -#include "minidriver.h"
> -#include "interface.h"
> +#include <jtag/jtag.h>
> +#include <jtag/interface.h>
>
> #ifdef HAVE_STRINGS_H
> #include <strings.h>
> @@ -398,18 +397,6 @@ void jtag_add_plain_ir_scan(int in_num_fields, const
> struct scan_field *in_field
> jtag_set_error(retval);
> }
>
> -void jtag_add_callback(jtag_callback1_t f, jtag_callback_data_t data0)
> -{
> - interface_jtag_add_callback(f, data0);
> -}
> -
> -void jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0,
> - jtag_callback_data_t data1, jtag_callback_data_t data2,
> - jtag_callback_data_t data3)
> -{
> - interface_jtag_add_callback4(f, data0, data1, data2, data3);
> -}
> -
> static int jtag_check_value_inner(uint8_t *captured, uint8_t *in_check_value,
> uint8_t *in_check_mask, int num_bits);
>
> @@ -491,20 +478,6 @@ void jtag_add_plain_dr_scan(int in_num_fields, const
> struct scan_field *in_field
> jtag_set_error(retval);
> }
>
> -void jtag_add_dr_out(struct jtag_tap* tap,
> - int num_fields, const int* num_bits, const uint32_t* value,
> - tap_state_t end_state)
> -{
> - assert(end_state != TAP_RESET);
> - assert(end_state != TAP_INVALID);
> -
> - cmd_queue_cur_state = end_state;
> -
> - interface_jtag_add_dr_out(tap,
> - num_fields, num_bits, value,
> - end_state);
> -}
> -
> void jtag_add_tlr(void)
> {
> jtag_prelude(TAP_RESET);
> diff --git a/src/jtag/drivers/driver.c b/src/jtag/drivers/driver.c
> index 7fa9ead..c57386a 100644
> --- a/src/jtag/drivers/driver.c
> +++ b/src/jtag/drivers/driver.c
> @@ -2,7 +2,7 @@
> * Copyright (C) 2005 by Dominic Rath *
> * dominic.r...@gmx.de *
> * *
> - * Copyright (C) 2007,2008 Øyvind Harboe *
> + * Copyright (C) 2007-2009 Øyvind Harboe *
> * oyvind.har...@zylin.com *
> * *
> * Copyright (C) 2009 SoftPLC Corporation *
> @@ -31,7 +31,9 @@
> #include "config.h"
> #endif
>
> +#include <jtag/jtag.h>
> #include <jtag/interface.h>
> +#include <jtag/commands.h>
> #include <jtag/minidriver.h>
> #include <helper/command.h>
>
> @@ -525,3 +527,30 @@ void interface_jtag_add_callback(jtag_callback1_t
> callback, jtag_callback_data_t
> jtag_add_callback4(jtag_convert_to_callback4, data0,
> (jtag_callback_data_t)callback, 0, 0);
> }
>
> +
> +/* A minidriver can use use an inline versions of this API level fn */
> +void jtag_add_dr_out(struct jtag_tap* tap,
> + int num_fields, const int* num_bits, const uint32_t* value,
> + tap_state_t end_state)
> +{
> + assert(end_state != TAP_RESET);
> + assert(end_state != TAP_INVALID);
> +
> + cmd_queue_cur_state = end_state;
> +
> + interface_jtag_add_dr_out(tap,
> + num_fields, num_bits, value,
> + end_state);
> +}
> +
> +void jtag_add_callback(jtag_callback1_t f, jtag_callback_data_t data0)
> +{
> + interface_jtag_add_callback(f, data0);
> +}
> +
> +void jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0,
> + jtag_callback_data_t data1, jtag_callback_data_t data2,
> + jtag_callback_data_t data3)
> +{
> + interface_jtag_add_callback4(f, data0, data1, data2, data3);
> +}
> diff --git a/src/jtag/drivers/minidriver_imp.h
> b/src/jtag/drivers/minidriver_imp.h
> index 1efd242..76cf9dd 100644
> --- a/src/jtag/drivers/minidriver_imp.h
> +++ b/src/jtag/drivers/minidriver_imp.h
> @@ -1,6 +1,6 @@
> /***************************************************************************
> * Copyright (C) 2005 by Dominic Rath <dominic.r...@gmx.de> *
> - * Copyright (C) 2007,2008 Øyvind Harboe <oyvind.har...@zylin.com> *
> + * Copyright (C) 2007-2009 Øyvind Harboe <oyvind.har...@zylin.com> *
> * Copyright (C) 2009 Zachary T Welch <z...@superlucidity.net>
> *
> * *
> * This program is free software; you can redistribute it and/or modify *
> @@ -44,4 +44,13 @@ void interface_jtag_add_callback4(jtag_callback_t f,
> jtag_callback_data_t data0,
> jtag_callback_data_t data1, jtag_callback_data_t data2,
> jtag_callback_data_t data3);
>
> +void jtag_add_dr_out(struct jtag_tap* tap,
> + int num_fields, const int* num_bits, const uint32_t* value,
> + tap_state_t end_state);
> +
> +
> +void jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0,
> + jtag_callback_data_t data1, jtag_callback_data_t data2,
> + jtag_callback_data_t data3);
> +
> #endif // MINIDRIVER_IMP_H
> diff --git a/src/jtag/jtag.h b/src/jtag/jtag.h
> index eaa0c7c..8c6fa44 100644
> --- a/src/jtag/jtag.h
> +++ b/src/jtag/jtag.h
> @@ -461,9 +461,6 @@ typedef int (*jtag_callback_t)(jtag_callback_data_t data0,
> * @param data3 An integer big enough to use as an @c int or a pointer.
> *
> */
> -void jtag_add_callback4(jtag_callback_t f, jtag_callback_data_t data0,
> - jtag_callback_data_t data1, jtag_callback_data_t data2,
> - jtag_callback_data_t data3);
>
>
> /**
> @@ -688,9 +685,7 @@ void jtag_sleep(uint32_t us);
> * There is no jtag_add_dr_outin() version of this fn that also allows
> * clocking data back in. Patches gladly accepted!
> */
> -void jtag_add_dr_out(struct jtag_tap* tap,
> - int num_fields, const int* num_bits, const uint32_t* value,
> - tap_state_t end_state);
> +
>
>
> /**
> @@ -725,4 +720,9 @@ bool jtag_poll_get_enabled(void);
> */
> void jtag_poll_set_enabled(bool value);
>
> +
> +/* The minidriver may have inline versions of some of the low
> + * level APIs that are used in inner loops. */
> +#include <jtag/minidriver.h>
> +
> #endif /* JTAG_H */
> diff --git a/src/jtag/minidriver.h b/src/jtag/minidriver.h
> index ea780fa..2109c75 100644
> --- a/src/jtag/minidriver.h
> +++ b/src/jtag/minidriver.h
> @@ -47,7 +47,7 @@
>
> // this header will be provided by the minidriver implementation,
> // and it may provide additional declarations that must be defined.
> -#include "minidriver_imp.h"
> +#include <jtag/minidriver_imp.h>
>
> int interface_jtag_add_ir_scan(
> int num_fields, const struct scan_field* fields,
> diff --git a/src/jtag/minidriver/minidriver_imp.h
> b/src/jtag/minidriver/minidriver_imp.h
> index e371514..b6cdbea 100644
> --- a/src/jtag/minidriver/minidriver_imp.h
> +++ b/src/jtag/minidriver/minidriver_imp.h
> @@ -21,7 +21,7 @@
> #ifndef MINIDRIVER_IMP_H
> #define MINIDRIVER_IMP_H
>
> -#include "jtag_minidriver.h"
> +#include <jtag/jtag_minidriver.h>
>
> static inline void interface_jtag_alloc_in_value32(struct scan_field *field)
> {
> @@ -41,4 +41,21 @@ static inline void
> interface_jtag_add_scan_check_alloc(struct scan_field *field)
> field->in_value = field->intmp;
> }
>
> +static inline void jtag_add_dr_out(struct jtag_tap* tap,
> + int num_fields, const int* num_bits, const uint32_t* value,
> + tap_state_t end_state)
> +{
> + cmd_queue_cur_state = end_state;
> +
> + interface_jtag_add_dr_out(tap,
> + num_fields, num_bits, value,
> + end_state);
> +}
> +
> +#define jtag_add_callback(callback, in)
> interface_jtag_add_callback(callback, in)
> +
> +#define jtag_add_callback4(callback, in, data1, data2, data3)
> interface_jtag_add_callback4(callback, in, data1, data2, data3)
> +
> +
> +
> #endif // MINIDRIVER_IMP_H
> diff --git a/src/jtag/minidummy/minidummy.c b/src/jtag/minidummy/minidummy.c
> index e60e832..9c608cd 100644
> --- a/src/jtag/minidummy/minidummy.c
> +++ b/src/jtag/minidummy/minidummy.c
> @@ -20,6 +20,7 @@
> #include "config.h"
> #endif
>
> +#include <jtag/jtag.h>
> #include <target/embeddedice.h>
> #include <jtag/minidriver.h>
> #include <jtag/interface.h>
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
