[RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[Julian Calaby]

Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
---
 arch/sparc/prom/console_32.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/prom/console_32.c b/arch/sparc/prom/console_32.c
index 4886310..b05e3db 100644
--- a/arch/sparc/prom/console_32.c
+++ b/arch/sparc/prom/console_32.c
@@ -27,13 +27,14 @@ static int prom_nbputchar(const char *buf)
 	spin_lock_irqsave(&prom_lock, flags);
 	switch(prom_vers) {
 	case PROM_V0:
-		i = (*(romvec->pv_nbputchar))(*buf);
+		if ((*(romvec->pv_nbputchar))(*buf))
+			i = 1;
 		break;
 	case PROM_V2:
 	case PROM_V3:
 		if ((*(romvec->pv_v2devops).v2_dev_write)(*romvec->pv_v2bootargs.fd_stdout,
 							  buf, 0x1) = 1)
-			i = 0;
+			i = 1;
 		break;
 	default:
 		break;
@@ -47,7 +48,7 @@ void prom_console_write_buf(const char *buf, int len)
 {
 	while (len) {
 		int n = prom_nbputchar(buf);
-		if (n)
+		if (n < 0)
 			continue;
 		len--;
 		buf++;
-- 
1.7.2.3

[RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[Julian Calaby]

Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
---

Sorry about the dupe, wrong from address.

 arch/sparc/prom/console_32.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/prom/console_32.c b/arch/sparc/prom/console_32.c
index 4886310..b05e3db 100644
--- a/arch/sparc/prom/console_32.c
+++ b/arch/sparc/prom/console_32.c
@@ -27,13 +27,14 @@ static int prom_nbputchar(const char *buf)
 	spin_lock_irqsave(&prom_lock, flags);
 	switch(prom_vers) {
 	case PROM_V0:
-		i = (*(romvec->pv_nbputchar))(*buf);
+		if ((*(romvec->pv_nbputchar))(*buf))
+			i = 1;
 		break;
 	case PROM_V2:
 	case PROM_V3:
 		if ((*(romvec->pv_v2devops).v2_dev_write)(*romvec->pv_v2bootargs.fd_stdout,
 							  buf, 0x1) = 1)
-			i = 0;
+			i = 1;
 		break;
 	default:
 		break;
@@ -47,7 +48,7 @@ void prom_console_write_buf(const char *buf, int len)
 {
 	while (len) {
 		int n = prom_nbputchar(buf);
-		if (n)
+		if (n < 0)
 			continue;
 		len--;
 		buf++;

Re: [RFC 1/8] sparc: prom: Sanitize return value from

[David Miller]

From: Julian Calaby <jcalaby@julian.is-a-geek.org>
Date: Sat, 04 Dec 2010 14:55:50 +1100

> Signed-off-by: Julian Calaby <julian.calaby@gmail.com>

Applied.

Re: [RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[Daniel Hellstrom]

David Miller wrote:

>From: Julian Calaby <jcalaby@julian.is-a-geek.org>
>Date: Sat, 04 Dec 2010 14:55:50 +1100
>
>  
>
>>Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
>>    
>>
>
>Applied.
>  
>

I think this patch breaks the backwards compatability... Before this 
patch one had to return -1 from prom-code in order to implement a 
non-locking nbputchar() in PROM code (see old code below). With the new 
patch however one has to return a 0 and -1 is now interpreted as send 
successful.

Should we not check for values bigger than 0 so that -1 can be return by 
old PROMs? I suggest:

-		i = (*(romvec->pv_nbputchar))(*buf);
+		if ((*(romvec->pv_nbputchar))(*buf) > 0)
+			i = 1;


Regards,
Daniel


The applied patch:

-		i = (*(romvec->pv_nbputchar))(*buf);
+		if ((*(romvec->pv_nbputchar))(*buf))
+			i = 1;


Old version of nbputchar():

/* Non blocking put character to console device, returns -1 if
 * unsuccessful.
 */
int
prom_nbputchar(char c)
{
	static char outc;
	unsigned long flags;
	int i = -1;

	spin_lock_irqsave(&prom_lock, flags);
	switch(prom_vers) {
	case PROM_V0:
		i = (*(romvec->pv_nbputchar))(c);
		break;
	case PROM_V2:
	case PROM_V3:
		outc = c;
		if( (*(romvec->pv_v2devops).v2_dev_write)(*romvec->pv_v2bootargs.fd_stdout, &outc, 0x1) = 1)
			i = 0;
		else
			i = -1;
		break;
	default:
		i = -1;
		break;
	};
	restore_current();
	spin_unlock_irqrestore(&prom_lock, flags);
	return i; /* Ugh, we could spin forever on unsupported proms ;( */
}

/* Blocking version of put character routine above. */
void
prom_putchar(char c)
{
	while(prom_nbputchar(c) = -1) ;
}

Re: [RFC 1/8] sparc: prom: Sanitize return value from

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Tue, 04 Jan 2011 15:42:45 +0100

> David Miller wrote:
> 
>>From: Julian Calaby <jcalaby@julian.is-a-geek.org>
>>Date: Sat, 04 Dec 2010 14:55:50 +1100
>>
>>  
>>>Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
>>>    
>>
>>Applied.
>>  
> 
> I think this patch breaks the backwards compatability...

Backwards compatability for who?  There is one and only one caller
of this function, and it is marked static and not exported to any
other piece of code in the tree.

Re: [RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[Julian Calaby]

On Wed, Jan 5, 2011 at 04:50, David Miller <davem@davemloft.net> wrote:
> From: Daniel Hellstrom <daniel@gaisler.com>
> Date: Tue, 04 Jan 2011 15:42:45 +0100
>
>> David Miller wrote:
>>
>>>From: Julian Calaby <jcalaby@julian.is-a-geek.org>
>>>Date: Sat, 04 Dec 2010 14:55:50 +1100
>>>
>>>
>>>>Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
>>>>
>>>
>>>Applied.
>>>
>>
>> I think this patch breaks the backwards compatability...
>
> Backwards compatability for who?  There is one and only one caller
> of this function, and it is marked static and not exported to any
> other piece of code in the tree.

The other point is that sparc64's version works like this, so why
should sparc32's version be different?

-- 

Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [RFC 1/8] sparc: prom: Sanitize return value from prom_nbputchar()

[Daniel Hellstrom]

Julian Calaby wrote:

>On Wed, Jan 5, 2011 at 04:50, David Miller <davem@davemloft.net> wrote:
>  
>
>>From: Daniel Hellstrom <daniel@gaisler.com>
>>Date: Tue, 04 Jan 2011 15:42:45 +0100
>>
>>    
>>
>>>David Miller wrote:
>>>
>>>      
>>>
>>>>From: Julian Calaby <jcalaby@julian.is-a-geek.org>
>>>>Date: Sat, 04 Dec 2010 14:55:50 +1100
>>>>
>>>>
>>>>        
>>>>
>>>>>Signed-off-by: Julian Calaby <julian.calaby@gmail.com>
>>>>>
>>>>>          
>>>>>
>>>>Applied.
>>>>
>>>>        
>>>>
>>>I think this patch breaks the backwards compatability...
>>>      
>>>
>>Backwards compatability for who?  There is one and only one caller
>>of this function, and it is marked static and not exported to any
>>other piece of code in the tree.
>>    
>>
With this patch we need to have two different versions of the PROM 
emulation implementation, it is a bit sad that it will not work for both 
versions of the linux kernel 2.6.36 and 2.6.37, that's all.

I'm probably out of line here, I havn't read the PROMv0 specification, 
the old linux implementation was perhaps faulty all along and we made 
our PROM code looking at it rather than the PROMv0 spec.

My impression was that pv_nbputchar() in PROM has to be implemented as 
non-blocking in order to avoid taking the prom_lock too long, in order 
to do that in a linux 2.6.36 kernel the PROM has to return -1 from 
pv_nbputchar() when the UART is busy and a 1 when pv_nbputchar() 
actually sends a character. However with this patch the pv_nbputchar() 
must return a 0 when UART is busy, this means that backwards compability 
to the PROM emultation software is lost.

>
>The other point is that sparc64's version works like this, so why
>should sparc32's version be different?
>  
>
Good point. Havn't looked at the SPARC64 code. Probably we should have 
submitted a patch for the old bad behaviour before.

We can live with two different versions of the PROM... there are more 
important things in life :)

Thank you,
Daniel