GNU bug report logs - #64068
[PATCH] gnu: Add u-boot-lichee-rv-dock.

Previous Next

Package: guix-patches;

Reported by: Z572 <873216071 <at> qq.com>

Date: Wed, 14 Jun 2023 14:10:01 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 64068 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to efraim <at> flashner.co.il, ludo <at> gnu.org, vagrant <at> debian.org, guix-patches <at> gnu.org:
bug#64068; Package guix-patches. (Wed, 14 Jun 2023 14:10:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Z572 <873216071 <at> qq.com>:
New bug report received and forwarded. Copy sent to efraim <at> flashner.co.il, ludo <at> gnu.org, vagrant <at> debian.org, guix-patches <at> gnu.org. (Wed, 14 Jun 2023 14:10:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Z572 <873216071 <at> qq.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add u-boot-lichee-rv-dock.
Date: Wed, 14 Jun 2023 22:09:13 +0800
* gnu/packages/bootloaders.scm (u-boot-lichee-rv-dock): New variable.
* gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch: New file.
* gnu/local.mk(dist_patch_DATA): register it.
---
 gnu/local.mk                                  |   1 +
 gnu/packages/bootloaders.scm                  |  39 ++++
 ...-boot-lichee-rv-dock-disable-openssl.patch | 216 ++++++++++++++++++
 3 files changed, 256 insertions(+)
 create mode 100644 gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 209556b56d..b2f4aaadc3 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1993,6 +1993,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/u-boot-allow-disabling-openssl.patch	\
   %D%/packages/patches/u-boot-fix-build-python-3.10.patch	\
   %D%/packages/patches/u-boot-infodocs-target.patch		\
+  %D%/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch	\
   %D%/packages/patches/u-boot-patman-guix-integration.patch	\
   %D%/packages/patches/u-boot-nintendo-nes-serial.patch		\
   %D%/packages/patches/u-boot-rockchip-inno-usb.patch		\
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index a9685a9ef9..4b80eaf3e8 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -18,6 +18,7 @@
 ;;; Copyright © 2022 Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
 ;;; Copyright © 2021 Stefan <stefan-guix <at> vodafonemail.de>
 ;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Cournoyer © 2023 Zheng Junjie <873216071 <at> qq.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1188,6 +1189,44 @@ (define-public u-boot-sifive-unmatched
        (modify-inputs (package-inputs base)
          (append opensbi-generic))))))
 
+(define-public u-boot-lichee-rv-dock
+  (let ((commit "528ae9bc6c55edd3ffe642734b4132a8246ea777")
+        (base (make-u-boot-package
+               "lichee_rv_dock"
+               "riscv64-linux-gnu")))
+    (package
+      (inherit base)
+      (source (origin
+                (inherit (package-source base))
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/smaeul/u-boot")
+                      (commit commit)))
+                (file-name (git-file-name
+                            (package-name base)
+                            (package-version base)))
+                (patches
+                 (append
+                  (search-patches
+                   "u-boot-lichee-rv-dock-disable-openssl.patch")
+                  ;;; rebase from %u-boot-allow-disabling-openssl-patch
+                  (filter (negate (cut string-contains <> "openssl"))
+                          (origin-patches (package-source base)))))
+                (sha256
+                 (base32
+                  "1rfk2d9wxxmf8ypvmwq07g1vifkvzy2nzs4mdwdgxsadlhi8dn9s"))))
+      (arguments
+       (substitute-keyword-arguments (package-arguments base)
+         ((#:phases phases)
+          #~(modify-phases #$phases
+              (add-after 'unpack 'set-environment
+                (lambda* (#:key inputs #:allow-other-keys)
+                  (setenv "OPENSBI" (search-input-file inputs
+                                                       "fw_dynamic.bin"))))))))
+      (inputs
+       (modify-inputs (package-inputs base)
+         (append opensbi-generic))))))
+
 (define-public u-boot-rock64-rk3328
   (let ((base (make-u-boot-package "rock64-rk3328" "aarch64-linux-gnu")))
     (package
diff --git a/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch b/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch
new file mode 100644
index 0000000000..f8e22d2c57
--- /dev/null
+++ b/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch
@@ -0,0 +1,216 @@
+From b2dff4fe9d1a53bbe3565435e190db19e7d6f4e7 Mon Sep 17 00:00:00 2001
+From: Z572 <zhengjunjie <at> iscas.ac.cn>
+Date: Mon, 8 May 2023 18:00:55 +0800
+Subject: [PATCH] remove openssl
+
+---
+ include/image.h    |  2 ++
+ tools/fit_image.c  |  3 ++-
+ tools/image-host.c |  4 ++++
+ tools/kwbimage.c   | 21 +++++++++++++++++++--
+ 4 files changed, 27 insertions(+), 3 deletions(-)
+
+diff --git a/include/image.h b/include/image.h
+index e5e12ce5..5c858a16 100644
+--- a/include/image.h
++++ b/include/image.h
+@@ -1172,6 +1172,7 @@ int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
+ 
+ int fit_set_timestamp(void *fit, int noffset, time_t timestamp);
+ 
++#ifdef CONFIG_FIT_PRELOAD
+ /**
+  * fit_pre_load_data() - add public key to fdt blob
+  *
+@@ -1186,6 +1187,7 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp);
+  *	< 0, on failure
+  */
+ int fit_pre_load_data(const char *keydir, void *keydest, void *fit);
++#endif
+ 
+ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
+ 		    const char *comment, int require_keys,
+diff --git a/tools/fit_image.c b/tools/fit_image.c
+index 923a9755..7a73f24a 100644
+--- a/tools/fit_image.c
++++ b/tools/fit_image.c
+@@ -59,9 +59,10 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
+ 		ret = fit_set_timestamp(ptr, 0, time);
+ 	}
+ 
++#ifdef CONFIG_FIT_PRELOAD
+ 	if (!ret)
+ 		ret = fit_pre_load_data(params->keydir, dest_blob, ptr);
+-
++#endif
+ 	if (!ret) {
+ 		ret = fit_cipher_data(params->keydir, dest_blob, ptr,
+ 				      params->comment,
+diff --git a/tools/image-host.c b/tools/image-host.c
+index 4e0512be..da70fcb4 100644
+--- a/tools/image-host.c
++++ b/tools/image-host.c
+@@ -14,9 +14,11 @@
+ #include <image.h>
+ #include <version.h>
+ 
++#ifdef CONFIG_FIT_PRELOAD
+ #include <openssl/pem.h>
+ #include <openssl/evp.h>
+ 
++#endif
+ /**
+  * fit_set_hash_value - set hash value in requested has node
+  * @fit: pointer to the FIT format image header
+@@ -1119,6 +1121,7 @@ static int fit_config_add_verification_data(const char *keydir,
+ 	return 0;
+ }
+ 
++#ifdef CONFIG_FIT_PRELOAD
+ /*
+  * 0) open file (open)
+  * 1) read certificate (PEM_read_X509)
+@@ -1227,6 +1230,7 @@ int fit_pre_load_data(const char *keydir, void *keydest, void *fit)
+  out:
+ 	return ret;
+ }
++#endif
+ 
+ int fit_cipher_data(const char *keydir, void *keydest, void *fit,
+ 		    const char *comment, int require_keys,
+diff --git a/tools/kwbimage.c b/tools/kwbimage.c
+index 6abb9f2d..d8159070 100644
+--- a/tools/kwbimage.c
++++ b/tools/kwbimage.c
+@@ -19,6 +19,7 @@
+ #include <stdint.h>
+ #include "kwbimage.h"
+ 
++#ifdef CONFIG_KWB_SECURE
+ #include <openssl/bn.h>
+ #include <openssl/rsa.h>
+ #include <openssl/pem.h>
+@@ -44,7 +45,7 @@ void EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx)
+ 	EVP_MD_CTX_reset(ctx);
+ }
+ #endif
+-
++#endif
+ /* fls - find last (most-significant) bit set in 4-bit integer */
+ static inline int fls4(int num)
+ {
+@@ -62,8 +63,9 @@ static inline int fls4(int num)
+ 
+ static struct image_cfg_element *image_cfg;
+ static int cfgn;
++#ifdef CONFIG_KWB_SECURE
+ static int verbose_mode;
+-
++#endif
+ struct boot_mode {
+ 	unsigned int id;
+ 	const char *name;
+@@ -278,6 +280,8 @@ image_count_options(unsigned int optiontype)
+ 	return count;
+ }
+ 
++#if defined(CONFIG_KWB_SECURE)
++
+ static int image_get_csk_index(void)
+ {
+ 	struct image_cfg_element *e;
+@@ -288,6 +292,7 @@ static int image_get_csk_index(void)
+ 
+ 	return e->csk_idx;
+ }
++#endif
+ 
+ static bool image_get_spezialized_img(void)
+ {
+@@ -432,6 +437,7 @@ static uint8_t baudrate_to_option(unsigned int baudrate)
+ 	}
+ }
+ 
++#if defined(CONFIG_KWB_SECURE)
+ static void kwb_msg(const char *fmt, ...)
+ {
+ 	if (verbose_mode) {
+@@ -926,6 +932,7 @@ static int kwb_dump_fuse_cmds(struct secure_hdr_v1 *sec_hdr)
+ done:
+ 	return ret;
+ }
++#endif
+ 
+ static size_t image_headersz_align(size_t headersz, uint8_t blockid)
+ {
+@@ -1079,11 +1086,13 @@ static size_t image_headersz_v1(int *hasext)
+ 	 */
+ 	headersz = sizeof(struct main_hdr_v1);
+ 
++#if defined(CONFIG_KWB_SECURE)
+ 	if (image_get_csk_index() >= 0) {
+ 		headersz += sizeof(struct secure_hdr_v1);
+ 		if (hasext)
+ 			*hasext = 1;
+ 	}
++#endif
+ 
+ 	cpu_sheeva = image_is_cpu_sheeva();
+ 
+@@ -1270,6 +1279,7 @@ err_close:
+ 	return -1;
+ }
+ 
++#if defined(CONFIG_KWB_SECURE)
+ static int export_pub_kak_hash(RSA *kak, struct secure_hdr_v1 *secure_hdr)
+ {
+ 	FILE *hashf;
+@@ -1382,6 +1392,7 @@ static int add_secure_header_v1(struct image_tool_params *params, uint8_t *ptr,
+ 
+ 	return 0;
+ }
++#endif
+ 
+ static void finish_register_set_header_v1(uint8_t **cur, uint8_t **next_ext,
+ 					  struct register_set_hdr_v1 *register_set_hdr,
+@@ -1406,7 +1417,9 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
+ 	struct main_hdr_v1 *main_hdr;
+ 	struct opt_hdr_v1 *ohdr;
+ 	struct register_set_hdr_v1 *register_set_hdr;
++#if defined(CONFIG_KWB_SECURE)
+ 	struct secure_hdr_v1 *secure_hdr = NULL;
++#endif
+ 	size_t headersz;
+ 	uint8_t *image, *cur;
+ 	int hasext = 0;
+@@ -1491,6 +1504,7 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
+ 	if (main_hdr->blockid == IBR_HDR_PEX_ID)
+ 		main_hdr->srcaddr = cpu_to_le32(0xFFFFFFFF);
+ 
++#if defined(CONFIG_KWB_SECURE)
+ 	if (image_get_csk_index() >= 0) {
+ 		/*
+ 		 * only reserve the space here; we fill the header later since
+@@ -1502,6 +1516,7 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
+ 		next_ext = &secure_hdr->next;
+ 	}
+ 
++#endif
+ 	datai = 0;
+ 	for (cfgi = 0; cfgi < cfgn; cfgi++) {
+ 		e = &image_cfg[cfgi];
+@@ -1552,9 +1567,11 @@ static void *image_create_v1(size_t *imagesz, struct image_tool_params *params,
+ 					      &datai, delay);
+ 	}
+ 
++#if defined(CONFIG_KWB_SECURE)
+ 	if (secure_hdr && add_secure_header_v1(params, ptr, payloadsz + headersz,
+ 					       headersz, image, secure_hdr))
+ 		return NULL;
++#endif
+ 
+ 	*imagesz = headersz;
+ 
+-- 
+2.39.2
+

base-commit: 7c12255eb9fdd73b481c3c588b02809abd633be8
-- 
2.40.1





Information forwarded to guix-patches <at> gnu.org:
bug#64068; Package guix-patches. (Fri, 16 Jun 2023 18:51:01 GMT) Full text and rfc822 format available.

Message #8 received at 64068 <at> debbugs.gnu.org (full text, mbox):

From: Vagrant Cascadian <vagrant <at> debian.org>
To: Z572 <873216071 <at> qq.com>, 64068 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Efraim Flashner <efraim <at> flashner.co.il>
Subject: Re: [bug#64068] [PATCH] gnu: Add u-boot-lichee-rv-dock.
Date: Fri, 16 Jun 2023 11:49:45 -0700
[Message part 1 (text/plain, inline)]
On 2023-06-14, Z572 wrote:
> * gnu/packages/bootloaders.scm (u-boot-lichee-rv-dock): New variable.
> * gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch: New file.
> * gnu/local.mk(dist_patch_DATA): register it.

Thanks for the patches!

This also reminds me that u-boot is falling a little behind (there is
2023.01, 2023.04 and soon 2023.07)... and while I would not want to block
adding support for this board, I would be curious if it still works with
newer u-boot versions...


> +(define-public u-boot-lichee-rv-dock
...
> +                  (search-patches
> +                   "u-boot-lichee-rv-dock-disable-openssl.patch")
> +                  ;;; rebase from %u-boot-allow-disabling-openssl-patch
> +                  (filter (negate (cut string-contains <> "openssl"))
> +                          (origin-patches (package-source base)))))

This seems a little tricky and possibly error-prone if another patch
with openssl in the name is included at a later time, it could break
this package. I almost wonder if it wouldn't be better to merge the
functionality of the two patches disabling openssl than applying a
board-specific patch?


I have not had a chance to test that this package builds; I presume you
have tested that it actually boots?


live well,
  vagrant
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#64068; Package guix-patches. (Sat, 17 Jun 2023 16:29:02 GMT) Full text and rfc822 format available.

Message #11 received at 64068 <at> debbugs.gnu.org (full text, mbox):

From: Z572 <873216071 <at> qq.com>
To: Vagrant Cascadian <vagrant <at> debian.org>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Efraim Flashner <efraim <at> flashner.co.il>, guix-patches <at> gnu.org,
 64068 <at> debbugs.gnu.org
Subject: Re: [bug#64068] [PATCH] gnu: Add u-boot-lichee-rv-dock.
Date: Sat, 17 Jun 2023 22:25:48 +0800
[Message part 1 (text/plain, inline)]
Vagrant Cascadian <vagrant <at> debian.org> writes:

> [[PGP Signed Part:Undecided]]
> On 2023-06-14, Z572 wrote:
>> * gnu/packages/bootloaders.scm (u-boot-lichee-rv-dock): New variable.
>> * gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch: New file.
>> * gnu/local.mk(dist_patch_DATA): register it.
>
> Thanks for the patches!
>
> This also reminds me that u-boot is falling a little behind (there is
> 2023.01, 2023.04 and soon 2023.07)... and while I would not want to block
> adding support for this board, I would be curious if it still works with
> newer u-boot versions...

for now is not upstreamed.

>
>
>> +(define-public u-boot-lichee-rv-dock
> ...
>> +                  (search-patches
>> +                   "u-boot-lichee-rv-dock-disable-openssl.patch")
>> +                  ;;; rebase from %u-boot-allow-disabling-openssl-patch
>> +                  (filter (negate (cut string-contains <> "openssl"))
>> +                          (origin-patches (package-source base)))))
>
> This seems a little tricky and possibly error-prone if another patch
> with openssl in the name is included at a later time, it could break
> this package. I almost wonder if it wouldn't be better to merge the
> functionality of the two patches disabling openssl than applying a
> board-specific patch?

this new patch is modified from the original patch, because original
patch can't apply to smaeul/u-boot.

I attach a new patch.

>
>
> I have not had a chance to test that this package builds; I presume you
> have tested that it actually boots?

yes, u-boot can boot, but cann't boot guix system, i guess because
initrd is too big(even though the board has 1GB of RAM), initrd.cpio is 60M. so i send this patch first,
and try "remove initrd" or "make initrd small" in future .
(I'm not familiar with u-boot)

I attach some photo, scm file and boot log.

https://wiki.sipeed.com/hardware/en/lichee/RV/Dock.html

[guix-riscv.scm (text/plain, attachment)]
[u-boot.png (image/png, attachment)]
[截图 2023-06-18 00-08-55.png (image/png, attachment)]
[955455260.jpg (image/jpeg, attachment)]
[Message part 6 (text/plain, inline)]
>
>
> live well,
>   vagrant
>
> [[End of PGP Signed Part]]


[0001-gnu-Add-u-boot-lichee-rv-dock.patch (text/x-patch, attachment)]
[lichee-rv-dock-boot.log (text/plain, attachment)]
[Message part 9 (text/plain, inline)]
-- 
over

Information forwarded to guix-patches <at> gnu.org:
bug#64068; Package guix-patches. (Sat, 17 Jun 2023 16:30:03 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#64068; Package guix-patches. (Sun, 02 Jul 2023 02:55:02 GMT) Full text and rfc822 format available.

Message #17 received at 64068 <at> debbugs.gnu.org (full text, mbox):

From: Vagrant Cascadian <vagrant <at> debian.org>
To: Z572 <873216071 <at> qq.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Efraim Flashner <efraim <at> flashner.co.il>, 64068 <at> debbugs.gnu.org
Subject: Re: [bug#64068] [PATCH] gnu: Add u-boot-lichee-rv-dock.
Date: Sat, 01 Jul 2023 19:54:41 -0700
[Message part 1 (text/plain, inline)]
On 2023-06-17, Z572 wrote:
> Vagrant Cascadian <vagrant <at> debian.org> writes:
>> On 2023-06-14, Z572 wrote:
>> This also reminds me that u-boot is falling a little behind (there is
>> 2023.01, 2023.04 and soon 2023.07)... and while I would not want to block
>> adding support for this board, I would be curious if it still works with
>> newer u-boot versions...
>
> for now is not upstreamed.

Ah, I missed that!

Since it is not from the upstream version, it should also specify the
version to match the version it is based on, even though it happens to
match right now (e.g. 2022.10)... Otherwise, once the u-boot it inherits
from is updated, I think it will list the package with the wrong
version...


>>> +(define-public u-boot-lichee-rv-dock
>> ...
>>> +                  (search-patches
>>> +                   "u-boot-lichee-rv-dock-disable-openssl.patch")
>>> +                  ;;; rebase from %u-boot-allow-disabling-openssl-patch
>>> +                  (filter (negate (cut string-contains <> "openssl"))
>>> +                          (origin-patches (package-source base)))))
>>
>> This seems a little tricky and possibly error-prone if another patch
>> with openssl in the name is included at a later time, it could break
>> this package. I almost wonder if it wouldn't be better to merge the
>> functionality of the two patches disabling openssl than applying a
>> board-specific patch?
>
> this new patch is modified from the original patch, because original
> patch can't apply to smaeul/u-boot.
>
> I attach a new patch.

Got it, thanks!

Once u-boot updates to 2023.07 or later, there will likely be more patch
conflicts... not sure of the best way to handle that...


>> I have not had a chance to test that this package builds; I presume you
>> have tested that it actually boots?
>
> yes, u-boot can boot, but cann't boot guix system, i guess because
> initrd is too big(even though the board has 1GB of RAM), initrd.cpio is 60M. so i send this patch first,
> and try "remove initrd" or "make initrd small" in future .
> (I'm not familiar with u-boot)

Hrm. Mixed feelings about getting it into guix at this point... with it
being a non-upstream fork and it cannot actually boot guix yet.


A few more comments:

> From: Z572 <873216071 <at> qq.com>
> Date: Sat, 17 Jun 2023 22:19:34 +0800
> Subject: [PATCH] gnu: Add u-boot-lichee-rv-dock.
> 
> * gnu/packages/bootloaders.scm (u-boot-lichee-rv-dock): New variable.
> * gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch: New file.
> * gnu/local.mk(dist_patch_DATA): register it.
...
> diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
> index a9685a9ef9..bd89280f1b 100644
> --- a/gnu/packages/bootloaders.scm
> +++ b/gnu/packages/bootloaders.scm
> @@ -18,6 +18,7 @@
>  ;;; Copyright © 2022 Denis 'GNUtoo' Carikli <GNUtoo <at> cyberdimension.org>
>  ;;; Copyright © 2021 Stefan <stefan-guix <at> vodafonemail.de>
>  ;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
> +;;; Cournoyer © 2023 Zheng Junjie <873216071 <at> qq.com>

I suspect you want "Copyright" not "Cournoyer" :)

> diff --git a/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch b/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch
> new file mode 100644
> index 0000000000..f8e22d2c57
> --- /dev/null
> +++ b/gnu/packages/patches/u-boot-lichee-rv-dock-disable-openssl.patch
> @@ -0,0 +1,216 @@
> +From b2dff4fe9d1a53bbe3565435e190db19e7d6f4e7 Mon Sep 17 00:00:00 2001
> +From: Z572 <zhengjunjie <at> iscas.ac.cn>
> +Date: Mon, 8 May 2023 18:00:55 +0800
> +Subject: [PATCH] remove openssl

If this is a rewrite of the removing openssl patch already in guix, it
should probably credit the original patch author...


live well,
  vagrant
[signature.asc (application/pgp-signature, inline)]

This bug report was last modified 307 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.