| Message ID | 4CF9BBC6.3080809@julian.is-a-geek.org |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) ; } -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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++;
Signed-off-by: Julian Calaby <julian.calaby@gmail.com> --- arch/sparc/prom/console_32.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)