All of lore.kernel.org
 help / color / mirror / Atom feed
* fix cg14 with serial console
@ 2005-02-01  4:37 Bob Breuer
  2005-02-01  4:44 ` David S. Miller
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Bob Breuer @ 2005-02-01  4:37 UTC (permalink / raw)
  To: sparclinux

When booting with a serial console and a vsimm installed, the cg14
driver would crash because the driver failed to check the return
value of prom_getproperty().

Here's a patch to fix that and move the SPARC32 conditional out of
the source file and into Kconfig.

Bob


diff -X dontdiff -urp linux-2.6.10-clean/drivers/video/cg14.c linux-2.6.10/drivers/video/cg14.c
--- linux-2.6.10-clean/drivers/video/cg14.c	2005-01-12 00:11:02.000000000 -0600
+++ linux-2.6.10/drivers/video/cg14.c	2005-01-31 22:31:36.000000000 -0600
@@ -469,9 +469,9 @@ static void cg14_init_one(struct sbus_de
  	int is_8mb, linebytes, i;

  	if (!sdev) {
-		prom_getproperty(node, "address",
-				 (char *) &bases[0], sizeof(bases));
-		if (!bases[0]) {
+		if (prom_getproperty(node, "address",
+				     (char *) &bases[0], sizeof(bases)) <= 0
+		    || !bases[0]) {
  			printk(KERN_ERR "cg14: Device is not mapped.\n");
  			return;
  		}
@@ -591,24 +591,20 @@ int __init cg14_init(void)
  {
  	struct sbus_bus *sbus;
  	struct sbus_dev *sdev;
+	int root, node;

  	if (fb_get_options("cg14fb", NULL))
  		return -ENODEV;

-#ifdef CONFIG_SPARC32
-	{
-		int root, node;
-
-		root = prom_getchild(prom_root_node);
-		root = prom_searchsiblings(root, "obio");
-		if (root) {
-			node = prom_searchsiblings(prom_getchild(root),
-						   "cgfourteen");
-			if (node)
-				cg14_init_one(NULL, node, root);
-		}
+	root = prom_getchild(prom_root_node);
+	root = prom_searchsiblings(root, "obio");
+	if (root) {
+		node = prom_searchsiblings(prom_getchild(root),
+					   "cgfourteen");
+		if (node)
+			cg14_init_one(NULL, node, root);
  	}
-#endif
+
  	for_all_sbusdev(sdev, sbus) {
  		if (!strcmp(sdev->prom_name, "cgfourteen"))
  			cg14_init_one(sdev, sdev->prom_node, sbus->prom_node);
diff -X dontdiff -urp linux-2.6.10-clean/drivers/video/Kconfig linux-2.6.10/drivers/video/Kconfig
--- linux-2.6.10-clean/drivers/video/Kconfig	2005-01-12 00:11:01.000000000 -0600
+++ linux-2.6.10/drivers/video/Kconfig	2005-01-31 22:11:23.000000000 -0600
@@ -954,14 +954,14 @@ config FB_FFB

  config FB_TCX
  	bool "TCX (SS4/SS5 only) support"
-	depends on FB_SBUS
+	depends on FB_SBUS && SPARC32
  	help
  	  This is the frame buffer device driver for the TCX 24/8bit frame
  	  buffer.

  config FB_CG14
  	bool "CGfourteen (SX) support"
-	depends on FB_SBUS
+	depends on FB_SBUS && SPARC32
  	help
  	  This is the frame buffer device driver for the CGfourteen frame
  	  buffer on Desktop SPARCsystems with the SX graphics option.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: fix cg14 with serial console
  2005-02-01  4:37 fix cg14 with serial console Bob Breuer
@ 2005-02-01  4:44 ` David S. Miller
  2005-02-05  4:04 ` Bob Breuer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2005-02-01  4:44 UTC (permalink / raw)
  To: sparclinux

On Mon, 31 Jan 2005 22:37:48 -0600
Bob Breuer <breuerr@mc.net> wrote:

> When booting with a serial console and a vsimm installed, the cg14
> driver would crash because the driver failed to check the return
> value of prom_getproperty().
> 
> Here's a patch to fix that and move the SPARC32 conditional out of
> the source file and into Kconfig.

Please don't do that so I can test the build of the
driver even under sparc64.  I made all the sparc FB
SBUS drivers available on sparc64 for this reason.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: fix cg14 with serial console
  2005-02-01  4:37 fix cg14 with serial console Bob Breuer
  2005-02-01  4:44 ` David S. Miller
@ 2005-02-05  4:04 ` Bob Breuer
  2005-02-05  5:41 ` David S. Miller
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Bob Breuer @ 2005-02-05  4:04 UTC (permalink / raw)
  To: sparclinux

David S. Miller wrote:
> Please don't do that so I can test the build of the
> driver even under sparc64.  I made all the sparc FB
> SBUS drivers available on sparc64 for this reason.

My bad, I didn't realize that was the case.  Some of these sparc
bits need as much help as they can get. :)  My intention was to
remove those ifdefs from the code.  I suppose the idea of creating
stubs for the missing prom functions on sparc64 has already been
considered and rejected.

Anyway, for this bug with the cg14, the return value of
prom_getproperty() was not checked.  A quick check of 2.6.10 shows
that the return value of prom_getproperty() is not used in 25 out
of 111 cases for sparc32.  Would it make sense to mark this
function with __must_check?  I suppose the cleanups would be too
much effort for too little gain.

And for good measure, I have appended a revised patch.

Bob


Fix crashing of the cg14 driver when booting with serial console
and a vsimm installed.

diff -X dontdiff -urp linux-2.6.10-clean/drivers/video/cg14.c linux-2.6.10/drivers/video/cg14.c
--- linux-2.6.10-clean/drivers/video/cg14.c	2005-01-12 00:11:02.000000000 -0600
+++ linux-2.6.10/drivers/video/cg14.c	2005-01-31 22:31:36.000000000 -0600
@@ -469,9 +469,9 @@ static void cg14_init_one(struct sbus_de
  	int is_8mb, linebytes, i;

  	if (!sdev) {
-		prom_getproperty(node, "address",
-				 (char *) &bases[0], sizeof(bases));
-		if (!bases[0]) {
+		if (prom_getproperty(node, "address",
+				     (char *) &bases[0], sizeof(bases)) <= 0
+		    || !bases[0]) {
  			printk(KERN_ERR "cg14: Device is not mapped.\n");
  			return;
  		}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: fix cg14 with serial console
  2005-02-01  4:37 fix cg14 with serial console Bob Breuer
  2005-02-01  4:44 ` David S. Miller
  2005-02-05  4:04 ` Bob Breuer
@ 2005-02-05  5:41 ` David S. Miller
  2005-02-06 22:23 ` Art Haas
  2005-02-15 18:06 ` David S. Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2005-02-05  5:41 UTC (permalink / raw)
  To: sparclinux

On Fri, 04 Feb 2005 22:04:38 -0600
Bob Breuer <breuerr@mc.net> wrote:

> I suppose the idea of creating stubs for the missing prom
> functions on sparc64 has already been considered and rejected.

Actually, this sounds like a nice cleanup idea.  It just hasn't
been done.

> Anyway, for this bug with the cg14, the return value of
> prom_getproperty() was not checked.  A quick check of 2.6.10 shows
> that the return value of prom_getproperty() is not used in 25 out
> of 111 cases for sparc32.  Would it make sense to mark this
> function with __must_check?  I suppose the cleanups would be too
> much effort for too little gain.

That would be a great idea.  In particular, things like
tree.c:prom_nodematch() should check (and return "0" when
error occurs).

> And for good measure, I have appended a revised patch.

Thanks Bob, I'll integrate.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: fix cg14 with serial console
  2005-02-01  4:37 fix cg14 with serial console Bob Breuer
                   ` (2 preceding siblings ...)
  2005-02-05  5:41 ` David S. Miller
@ 2005-02-06 22:23 ` Art Haas
  2005-02-15 18:06 ` David S. Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Art Haas @ 2005-02-06 22:23 UTC (permalink / raw)
  To: sparclinux

On Fri, Feb 04, 2005 at 09:41:42PM -0800, David S. Miller wrote:
> On Fri, 04 Feb 2005 22:04:38 -0600
> Bob Breuer <breuerr@mc.net> wrote:
> > Anyway, for this bug with the cg14, the return value of
> > prom_getproperty() was not checked.  A quick check of 2.6.10 shows
> > that the return value of prom_getproperty() is not used in 25 out
> > of 111 cases for sparc32.  Would it make sense to mark this
> > function with __must_check?  I suppose the cleanups would be too
> > much effort for too little gain.
> 
> That would be a great idea.  In particular, things like
> tree.c:prom_nodematch() should check (and return "0" when
> error occurs).

How does this look? It compiled but is untested in a running kernel.

Art Haas

=== arch/sparc/prom/tree.c 1.1 vs edited ==--- 1.1/arch/sparc/prom/tree.c	2002-02-05 11:40:23 -06:00
+++ edited/arch/sparc/prom/tree.c	2005-02-06 16:18:29 -06:00
@@ -176,8 +176,11 @@
  */
 int prom_nodematch(int node, char *name)
 {
+	int error;
+
 	static char namebuf[128];
-	prom_getproperty(node, "name", namebuf, sizeof(namebuf));
+	error = prom_getproperty(node, "name", namebuf, sizeof(namebuf));
+	if (error = -1) return 0;
 	if(strcmp(namebuf, name) = 0) return 1;
 	return 0;
 }

-- 
Man once surrendering his reason, has no remaining guard against absurdities
the most monstrous, and like a ship without rudder, is the sport of every wind.

-Thomas Jefferson to James Smith, 1822

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: fix cg14 with serial console
  2005-02-01  4:37 fix cg14 with serial console Bob Breuer
                   ` (3 preceding siblings ...)
  2005-02-06 22:23 ` Art Haas
@ 2005-02-15 18:06 ` David S. Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David S. Miller @ 2005-02-15 18:06 UTC (permalink / raw)
  To: sparclinux

On Sun, 6 Feb 2005 16:23:39 -0600
"Art Haas" <ahaas@airmail.net> wrote:

> On Fri, Feb 04, 2005 at 09:41:42PM -0800, David S. Miller wrote:
> > That would be a great idea.  In particular, things like
> > tree.c:prom_nodematch() should check (and return "0" when
> > error occurs).
> 
> How does this look? It compiled but is untested in a running kernel.

Looks great, patch applied.  Thanks Art.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-02-15 18:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-01  4:37 fix cg14 with serial console Bob Breuer
2005-02-01  4:44 ` David S. Miller
2005-02-05  4:04 ` Bob Breuer
2005-02-05  5:41 ` David S. Miller
2005-02-06 22:23 ` Art Haas
2005-02-15 18:06 ` David S. Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.