[PATCH] dsp56k: use request_firmware

[PATCH] dsp56k: use request_firmware

[Jaswinder Singh]

Signed-off-by: Jaswinder Singh <jaswinder@infradead.org>
---
 drivers/char/dsp56k.c              |   84 ++++++++++++++----------------------
 firmware/Makefile                  |    1 +
 firmware/WHENCE                    |   10 ++++
 firmware/dsp56k/bootstrap.bin.ihex |   26 +++++++++++
 4 files changed, 70 insertions(+), 51 deletions(-)
 create mode 100644 firmware/dsp56k/bootstrap.bin.ihex

diff --git a/drivers/char/dsp56k.c b/drivers/char/dsp56k.c
index a69c652..88f55b5 100644
--- a/drivers/char/dsp56k.c
+++ b/drivers/char/dsp56k.c
@@ -33,6 +33,8 @@
 #include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/platform_device.h>
 
 #include <asm/atarihw.h>
 #include <asm/traps.h>
@@ -92,49 +94,6 @@
 	} \
 }
 
-/* DSP56001 bootstrap code */
-static char bootstrap[] = {
-	0x0c, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x60, 0xf4, 0x00, 0x00, 0x00, 0x4f, 0x61, 0xf4,
-	0x00, 0x00, 0x7e, 0xa9, 0x06, 0x2e, 0x80, 0x00, 0x00, 0x47,
-	0x07, 0xd8, 0x84, 0x07, 0x59, 0x84, 0x08, 0xf4, 0xa8, 0x00,
-	0x00, 0x04, 0x08, 0xf4, 0xbf, 0x00, 0x0c, 0x00, 0x00, 0xfe,
-	0xb8, 0x0a, 0xf0, 0x80, 0x00, 0x7e, 0xa9, 0x08, 0xf4, 0xa0,
-	0x00, 0x00, 0x01, 0x08, 0xf4, 0xbe, 0x00, 0x00, 0x00, 0x0a,
-	0xa9, 0x80, 0x00, 0x7e, 0xad, 0x08, 0x4e, 0x2b, 0x44, 0xf4,
-	0x00, 0x00, 0x00, 0x03, 0x44, 0xf4, 0x45, 0x00, 0x00, 0x01,
-	0x0e, 0xa0, 0x00, 0x0a, 0xa9, 0x80, 0x00, 0x7e, 0xb5, 0x08,
-	0x50, 0x2b, 0x0a, 0xa9, 0x80, 0x00, 0x7e, 0xb8, 0x08, 0x46,
-	0x2b, 0x44, 0xf4, 0x45, 0x00, 0x00, 0x02, 0x0a, 0xf0, 0xaa,
-	0x00, 0x7e, 0xc9, 0x20, 0x00, 0x45, 0x0a, 0xf0, 0xaa, 0x00,
-	0x7e, 0xd0, 0x06, 0xc6, 0x00, 0x00, 0x7e, 0xc6, 0x0a, 0xa9,
-	0x80, 0x00, 0x7e, 0xc4, 0x08, 0x58, 0x6b, 0x0a, 0xf0, 0x80,
-	0x00, 0x7e, 0xad, 0x06, 0xc6, 0x00, 0x00, 0x7e, 0xcd, 0x0a,
-	0xa9, 0x80, 0x00, 0x7e, 0xcb, 0x08, 0x58, 0xab, 0x0a, 0xf0,
-	0x80, 0x00, 0x7e, 0xad, 0x06, 0xc6, 0x00, 0x00, 0x7e, 0xd4,
-	0x0a, 0xa9, 0x80, 0x00, 0x7e, 0xd2, 0x08, 0x58, 0xeb, 0x0a,
-	0xf0, 0x80, 0x00, 0x7e, 0xad};
-static int sizeof_bootstrap = 375;
-
-
 static struct dsp56k_device {
 	unsigned long in_use;
 	long maxio, timeout;
@@ -164,18 +123,40 @@ static int dsp56k_reset(void)
 
 static int dsp56k_upload(u_char __user *bin, int len)
 {
+	struct platform_device *pdev;
+	const struct firmware *fw;
+	const char fw_name[] = "dsp56k/bootstrap.bin";
+	int err;
 	int i;
-	u_char *p;
-	
+
 	dsp56k_reset();
-  
-	p = bootstrap;
-	for (i = 0; i < sizeof_bootstrap/3; i++) {
+
+	pdev = platform_device_register_simple("dsp56k", 0, NULL, 0);
+	if (IS_ERR(pdev)) {
+		printk(KERN_ERR "Failed to register device for \"%s\"\n",
+		       fw_name);
+		return -EINVAL;
+	}
+	err = request_firmware(&fw, fw_name, &pdev->dev);
+	platform_device_unregister(pdev);
+	if (err) {
+		printk(KERN_ERR "Failed to load image \"%s\" err %d\n",
+		       fw_name, err);
+		return err;
+	}
+	if (fw->size % 3) {
+		printk(KERN_ERR "Bogus length %d in image \"%s\"\n",
+		       fw->size, fw_name);
+		release_firmware(fw);
+		return -EINVAL;
+	}
+	for (i = 0; i < fw->size; i + 3) {
 		/* tx_wait(10); */
-		dsp56k_host_interface.data.b[1] = *p++;
-		dsp56k_host_interface.data.b[2] = *p++;
-		dsp56k_host_interface.data.b[3] = *p++;
+		dsp56k_host_interface.data.b[1] = fw->data[i];
+		dsp56k_host_interface.data.b[2] = fw->data[i + 1];
+		dsp56k_host_interface.data.b[3] = fw->data[i + 2];
 	}
+	release_firmware(fw);
 	for (; i < 512; i++) {
 		/* tx_wait(10); */
 		dsp56k_host_interface.data.b[1] = 0;
@@ -534,3 +515,4 @@ static void __exit dsp56k_cleanup_driver(void)
 module_exit(dsp56k_cleanup_driver);
 
 MODULE_LICENSE("GPL");
+MODULE_FIRMWARE("dsp56k/bootstrap.bin");
diff --git a/firmware/Makefile b/firmware/Makefile
index 0f91f3f..58ee6ed 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -13,6 +13,7 @@ ifneq ($(CONFIG_ACENIC_OMIT_TIGON_I),y)
 fw-shipped-$(CONFIG_ACENIC) += acenic/tg1.bin
 endif
 fw-shipped-$(CONFIG_ACENIC) += acenic/tg2.bin
+fw-shipped-$(CONFIG_ATARI_DSP56K) += dsp56k/bootstrap.bin
 fw-shipped-$(CONFIG_ATM_AMBASSADOR) += atmsar11.fw
 fw-shipped-$(CONFIG_COMPUTONE) += intelliport2.bin
 fw-shipped-$(CONFIG_DVB_AV7110) += av7110/bootcode.bin
diff --git a/firmware/WHENCE b/firmware/WHENCE
index 4c120b3..dd4ead5 100644
--- a/firmware/WHENCE
+++ b/firmware/WHENCE
@@ -362,3 +362,13 @@ Licence:
 Found in hex form in kernel source.
 
 --------------------------------------------------------------------------
+
+Driver: ATARI_DSP56K - Atari DSP56k support
+
+File: dsp56k/bootstap.bin
+
+Licence: Unknown
+
+Found in hex form in kernel source.
+
+--------------------------------------------------------------------------
diff --git a/firmware/dsp56k/bootstrap.bin.ihex b/firmware/dsp56k/bootstrap.bin.ihex
new file mode 100644
index 0000000..233f21f
--- /dev/null
+++ b/firmware/dsp56k/bootstrap.bin.ihex
@@ -0,0 +1,26 @@
+:100000000C004000000000000000000000000000A4
+:1000100000000000000000000000000000000000E0
+:1000200000000000000000000000000000000000D0
+:1000300000000000000000000000000000000000C0
+:1000400000000000000000000000000000000000B0
+:1000500000000000000000000000000000000000A0
+:100060000000000000000000000000000000000090
+:100070000000000000000000000000000000000080
+:100080000000000000000000000000000000000070
+:100090000000000000000000000000000000000060
+:1000A0000000000000000000000000000000000050
+:1000B0000000000000000000000000000000000040
+:1000C00060F40000004F61F400007EA9062E80005D
+:1000D000004707D88407598408F4A800000408F4EE
+:1000E000BF000C0000FEB80AF080007EA908F4A052
+:1000F00000000108F4BE0000000AA980007EAD08DF
+:100100004E2B44F40000000344F4450000010EA00F
+:10011000000AA980007EB508502B0AA980007EB88D
+:1001200008462B44F4450000020AF0AA007EC920CC
+:1001300000450AF0AA007ED006C600007EC60AA9C5
+:1001400080007EC408586B0AF080007EAD06C600B1
+:10015000007ECD0AA980007ECB0858AB0AF0800053
+:100160007EAD06C600007ED40AA980007ED2085863
+:07017000EB0AF080007EADF8
+:00000001FF
+/* DSP56001 bootstrap code */
-- 
1.5.5.1

Re: [PATCH] dsp56k: use request_firmware

[Fredrik Noring]

Hi Jaswinder,

5 jul 2008 kl. 12.24 skrev Jaswinder Singh:
> +	for (i = 0; i < fw->size; i + 3) {

Are you sure about "i + 3" in this loop? Isn't GCC complaining,  
something like "statement with no effect"?

> +Driver: ATARI_DSP56K - Atari DSP56k support
> +
> +File: dsp56k/bootstap.bin
> +
> +Licence: Unknown
> +
> +Found in hex form in kernel source.

I wrote this DSP56k assembler code in 1995. It's a trivial copy loop  
that loads the main DSP program into memory. Please consider it GPL  
like the driver code it came from.

A disassembler would easily recreate the original ~50 lines of  
assembler source code, in case you'd like to assemble it with the rest  
of kernel. :)

Thanks,
Fredrik

Re: [PATCH] dsp56k: use request_firmware

[Jaswinder Singh]

Hello Fredrik,

On Sun, 2008-07-06 at 15:36 +0200, Fredrik Noring wrote:
> Hi Jaswinder,
> 
> 5 jul 2008 kl. 12.24 skrev Jaswinder Singh:
> > +	for (i = 0; i < fw->size; i + 3) {
> 
> Are you sure about "i + 3" in this loop? Isn't GCC complaining,  
> something like "statement with no effect"?
> 

Fixed, Thanks.

> > +Driver: ATARI_DSP56K - Atari DSP56k support
> > +
> > +File: dsp56k/bootstap.bin
> > +
> > +Licence: Unknown
> > +
> > +Found in hex form in kernel source.
> 
> I wrote this DSP56k assembler code in 1995. It's a trivial copy loop  
> that loads the main DSP program into memory. Please consider it GPL  
> like the driver code it came from.
> 
> A disassembler would easily recreate the original ~50 lines of  
> assembler source code, in case you'd like to assemble it with the rest  
> of kernel. :)
> 

Fixed.

Updated patch :
http://git.infradead.org/users/jaswinder/firm-jsr-2.6.git?a=commitdiff;h=4f5b8828113ea857a7b590b7dbb8a5ee78103de4

Thanks for you comments.

Thank you,

Jaswinder Singh.

Re: [PATCH] dsp56k: use request_firmware

[Fredrik Noring]

Hi Jaswinder,

6 jul 2008 kl. 17.13 skrev Jaswinder Singh:
> Updated patch :
> http://git.infradead.org/users/jaswinder/firm-jsr-2.6.git?a=commitdiff;h=4f5b8828113ea857a7b590b7dbb8a5ee78103de4
>
> Thanks for you comments.

Excellent. (Unfortunately I don't have access to the hardware to test  
these changes.)

Many thanks,
Fredrik

Re: [PATCH] dsp56k: use request_firmware

[Jaswinder Singh]

Hello Fredrik,

On Sun, 2008-07-06 at 17:36 +0200, Fredrik Noring wrote:
> Hi Jaswinder,
> 
> 6 jul 2008 kl. 17.13 skrev Jaswinder Singh:
> > Updated patch :
> > http://git.infradead.org/users/jaswinder/firm-jsr-2.6.git?a=commitdiff;h=4f5b8828113ea857a7b590b7dbb8a5ee78103de4
> >
> > Thanks for you comments.
> 
> Excellent. (Unfortunately I don't have access to the hardware to test  
> these changes.)

Please test this and let us know the result.

Please upload assembly code with Licence on Net so that I can point
Assembly Source code in firmware/WHENCE.

Thank you,

Jaswinder Singh.

Re: [PATCH] dsp56k: use request_firmware

[David Woodhouse]

On Sun, 2008-07-06 at 21:08 +0530, Jaswinder Singh wrote:
> Hello Fredrik,
> 
> On Sun, 2008-07-06 at 17:36 +0200, Fredrik Noring wrote:
> > Hi Jaswinder,
> > 
> > 6 jul 2008 kl. 17.13 skrev Jaswinder Singh:
> > > Updated patch :
> > > http://git.infradead.org/users/jaswinder/firm-jsr-2.6.git?a=commitdiff;h=4f5b8828113ea857a7b590b7dbb8a5ee78103de4


+Licence: GPL
+Assembly Source Code of ~50 lines can be generated by disassemble

Shouldn't that be 'GPLv2'? 

And it's better just to say '...but original assembler source code lost'
than to point say that it can be disassembled.

It would be better still to dig out a DSP56001 disassembler and actually
disassemble it. Why don't you do that?

> > > Thanks for you comments.
> > 
> > Excellent. (Unfortunately I don't have access to the hardware to test  
> > these changes.)
> 
> Please test this and let us know the result.

He said he can't.

> Please upload assembly code with Licence on Net so that I can point
> Assembly Source code in firmware/WHENCE.

I don't think he has it. That's why he suggested a disassembler.

-- 
dwmw2

Re: [PATCH] dsp56k: use request_firmware

[Fredrik Noring]

Hi David,

6 jul 2008 kl. 18.54 skrev David Woodhouse:
> I don't think he has it. That's why he suggested a disassembler.

That's right. Sorry I was unclear. However, after some digging on  
floppy backups from the past, I've found a DSP56k loader routine  
that's probably close if not identical to the assembled version  
included with the kernel. Please see below. A disassembly would verify  
this, but at least it gives you an idea of the inner workings.

All the best,
Fredrik

; DSP56k loader

; Host Interface
M_BCR   EQU     $FFFE           ; Port A Bus Control Register
M_PBC   EQU     $FFE0           ; Port B Control Register
M_PBDDR EQU     $FFE2           ; Port B Data Direction Register
M_PBD   EQU     $FFE4           ; Port B Data Register
M_PCC   EQU     $FFE1           ; Port C Control Register
M_PCDDR EQU     $FFE3           ; Port C Data Direction Register
M_PCD   EQU     $FFE5           ; Port C Data Register

M_HCR   EQU     $FFE8           ; Host Control Register
M_HSR   EQU     $FFE9           ; Host Status Register
M_HRX   EQU     $FFEB           ; Host Receive Data Register
M_HTX   EQU     $FFEB           ; Host Transmit Data Register

; SSI, Synchronous Serial Interface
M_RX    EQU     $FFEF           ; Serial Receive Data Register
M_TX    EQU     $FFEF           ; Serial Transmit Data Register
M_CRA   EQU     $FFEC           ; SSI Control Register A
M_CRB   EQU     $FFED           ; SSI Control Register B
M_SR    EQU     $FFEE           ; SSI Status Register
M_TSR   EQU     $FFEE           ; SSI Time Slot Register

; Exception Processing
M_IPR   EQU     $FFFF           ; Interrupt Priority Register

	org	P:$0
start	jmp	<$40

	org	P:$40
;	; Zero 16384 DSP X and Y words
;	clr	A #0,r0
;	clr	B #0,r4
;	do	#64,<_block1
;	rep	#256
;	move	A,X:(r0)+ B,Y:(r4)+
;_block1	; Zero (32768-512) Program words
;	clr	A #512,r0
;	do	#126,<_block2
;	rep	#256
;	move	A,P:(r0)+
;_block2

	; Copy DSP program control
	move	#real,r0
	move	#upload,r1
	do	#upload_end-upload,<_copy
	move	P:(r0)+,x0
	move	x0,P:(r1)+
_copy	movep	#>4,X:<<M_HCR
	movep	#>$c00,X:<<M_IPR
	and	#<$fe,mr
	jmp	upload

real
	org	P:$7ea9
upload
	movep	#>1,X:<<M_PBC
	movep	#>0,X:<<M_BCR

next	jclr	#0,X:<<M_HSR,*
	movep	X:<<M_HRX,A
	move	#>3,x0
	cmp	x0,A #>1,x0
	jeq	<$0
_get_address
	jclr	#0,X:<<M_HSR,_get_address
	movep	X:<<M_HRX,r0
_get_length
	jclr	#0,X:<<M_HSR,_get_length
	movep	X:<<M_HRX,y0
	cmp	x0,A #>2,x0
	jeq	load_X
	cmp	x0,A
	jeq	load_Y

load_P	do	y0,_load
	jclr	#0,X:<<M_HSR,*
	movep	X:<<M_HRX,P:(r0)+
_load	jmp	next
load_X	do	y0,_load
	jclr	#0,X:<<M_HSR,*
	movep	X:<<M_HRX,X:(r0)+
_load	jmp	next
load_Y	do	y0,_load
	jclr	#0,X:<<M_HSR,*
	movep	X:<<M_HRX,Y:(r0)+
_load	jmp	next

upload_end
	end

Re: [PATCH] dsp56k: use request_firmware

[David Woodhouse]

On Sun, 2008-07-06 at 20:29 +0200, Fredrik Noring wrote:
> That's right. Sorry I was unclear. However, after some digging on  
> floppy backups from the past, I've found a DSP56k loader routine  
> that's probably close if not identical to the assembled version  
> included with the kernel. Please see below. A disassembly would verify
> this, but at least it gives you an idea of the inner workings.

It is in fact identical. Disassembly with the tools found on sourceforge
was fun, since they have endianness bugs, don't recognise the 'JMP'
which was the first instruction, and need each 3-byte instruction in the
binary to be prefixed with a zero byte to make it 4 bytes. But after
sorting that out and going through it instruction by instruction, it
really does seem to match. Thanks very much for digging it out.

When you say 'GPL like the driver it came from', actually the driver
doesn't specify any version of the GPL but does refer to the top-level
COPYING file. So we'll take that to mean 'GPLv2 and not later', like the
majority of the kernel. OK?

-- 
dwmw2

Re: [PATCH] dsp56k: use request_firmware

[Fredrik Noring]

Hi David,

9 jul 2008 kl. 15.05 skrev David Woodhouse:
> It is in fact identical. Disassembly with the tools found on  
> sourceforge
> was fun, since they have endianness bugs, don't recognise the 'JMP'
> which was the first instruction, and need each 3-byte instruction in  
> the
> binary to be prefixed with a zero byte to make it 4 bytes. But after
> sorting that out and going through it instruction by instruction, it
> really does seem to match.

Heh. Amazing.

[ I think the code can be simplified: It currently copies itself to a  
high program memory location, but this is not necessary because DSP  
soft reset was never implemented (the reset would have been trapped by  
the code, in order to load a new program). The driver instead power- 
cycles the DSP every time a new program is loaded. ]

> Thanks very much for digging it out.

No problem!

> When you say 'GPL like the driver it came from', actually the driver
> doesn't specify any version of the GPL but does refer to the top-level
> COPYING file. So we'll take that to mean 'GPLv2 and not later', like  
> the
> majority of the kernel. OK?

OK with me.

Thanks,
Fredrik

Re: [PATCH] dsp56k: use request_firmware

[David Woodhouse]

On Thu, 2008-07-10 at 09:38 +0200, Fredrik Noring wrote:
> Hi David,
> 
> 9 jul 2008 kl. 15.05 skrev David Woodhouse:
> > It is in fact identical. Disassembly with the tools found on  
> > sourceforge
> > was fun, since they have endianness bugs, don't recognise the 'JMP'
> > which was the first instruction, and need each 3-byte instruction in  
> > the
> > binary to be prefixed with a zero byte to make it 4 bytes. But after
> > sorting that out and going through it instruction by instruction, it
> > really does seem to match.
> 
> Heh. Amazing.
> 
> [ I think the code can be simplified: It currently copies itself to a  
> high program memory location, but this is not necessary because DSP  
> soft reset was never implemented (the reset would have been trapped by  
> the code, in order to load a new program). The driver instead power- 
> cycles the DSP every time a new program is loaded. ]

I did wonder about that. But since we can't test, we're not going to
touch. Better to keep the assembler code which precisely matches the
known-good binary.

-- 
dwmw2