From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1Ifh5r-0008Kk-9f for mharc-grub-devel@gnu.org; Wed, 10 Oct 2007 15:19:43 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ifh5p-0008Jb-Qh for grub-devel@gnu.org; Wed, 10 Oct 2007 15:19:41 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ifh5k-0008E1-FH for grub-devel@gnu.org; Wed, 10 Oct 2007 15:19:40 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ifh5k-0008Dr-8s for grub-devel@gnu.org; Wed, 10 Oct 2007 15:19:36 -0400 Received: from aybabtu.com ([69.60.117.155]) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Ifh5k-0002lU-1t for grub-devel@gnu.org; Wed, 10 Oct 2007 15:19:36 -0400 Received: from [192.168.10.6] (helo=thorin) by aybabtu.com with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1Ifh5h-0003fB-Ez for grub-devel@gnu.org; Wed, 10 Oct 2007 21:19:34 +0200 Received: from rmh by thorin with local (Exim 4.63) (envelope-from ) id 1Ifh5K-0007Fj-6X for grub-devel@gnu.org; Wed, 10 Oct 2007 21:19:10 +0200 Date: Wed, 10 Oct 2007 21:19:10 +0200 From: Robert Millan To: The development of GRUB 2 Message-ID: <20071010191910.GA26777@thorin> References: <8a2a06bb0710010357u142f6ba4gb053afde829f0f39@mail.gmail.com> <20071001123345.GB20428@thorin> <8a2a06bb0710011114l5cc66483vc27b0b83eb3395e1@mail.gmail.com> <20071001183927.GA10233@thorin> <8a2a06bb0710011243m38ac71a0t463b6ea56ff19e78@mail.gmail.com> <20071002213942.GC29487@thorin> <8a2a06bb0710031633v55549654v9184c3a460c202f5@mail.gmail.com> <20071004205038.GB23726@thorin> <8a2a06bb0710100811m41e08028t2404090a4d9a91b9@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8a2a06bb0710100811m41e08028t2404090a4d9a91b9@mail.gmail.com> Organization: free as in freedom X-Message-Flag: Microsoft discourages use of Outlook. X-Debbugs-No-Ack: true User-Agent: Mutt/1.5.13 (2006-08-11) X-Detected-Kernel: Genre and OS details not recognized. Subject: Re: [PATCH] Fixed ieee1275 console X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Oct 2007 19:19:42 -0000 On Wed, Oct 10, 2007 at 05:11:13PM +0200, Marcin Kurek wrote: > Hell[o] > > > Avoid the cosmetical changes! (there are more thorough the patch) > > Arghh, I was sure I removed it :/ Anyway attached updated console patches. Great, thank you. > diff -urN grub2.org/term/ieee1275/ofconsole.c grub2/term/ieee1275/ofconsole.c > --- grub2.org/term/ieee1275/ofconsole.c 2007-07-22 11:05:11.000000000 +0200 > +++ grub2/term/ieee1275/ofconsole.c 2007-10-10 16:03:34.136688149 +0200 > @@ -75,6 +75,7 @@ > grub_ofconsole_putchar (grub_uint32_t c) > { > char chr = c; > + > if (c == '\n') Caught you! ;-) > diff -urN grub2.org/include/grub/ieee1275/ieee1275.h grub2/include/grub/ieee1275/ieee1275.h > --- grub2.org/include/grub/ieee1275/ieee1275.h 2007-10-04 22:44:12.000000000 +0200 > +++ grub2/include/grub/ieee1275/ieee1275.h 2007-10-10 16:41:39.594688149 +0200 > @@ -82,6 +82,16 @@ > > /* CodeGen firmware does not correctly implement "output-device output" */ > GRUB_IEEE1275_FLAG_BROKEN_OUTPUT, > + > + /* In non fb mode default number of console rows is 24, but in fact it's 25 */ > + GRUB_IEEE1275_FLAG_NOFB_ROWS25, > + > + /* Old Pegaos firmware does not accept cls escape sequence */ > + GRUB_IEEE1275_FLAG_NOCLS, > + > + /* On CodeGen firmware, cp437 characters 0xc0 to 0xcb are reserved for the > + bplan logo */ > + GRUB_IEEE1275_FLAG_BPLAN_LOGO, > }; I know it seems burdensome, but please can you split this in three patches, one for each fix? Then it's easier to review just one and say "this is good", and also easier to figure out why every hunk was done (since one knows what we're trying to archieve). > + /* It seems no cls escape is available then simulate it using \n flood */ > + int x = (grub_ofconsole_height * 2) - grub_curr_y; Maybe this would be easier to understand as "(grub_ofconsole_height - grub_curr_y) + grub_ofconsole_height". What do others think? :-) > + /* Check do we are on serial or normal console */ > + if(! grub_ieee1275_instance_to_package (stdout_ihandle, &stdout_phandle)) > + { > + char type[16]; > + char name[128]; > + > + if(! grub_ieee1275_get_property (stdout_phandle, "device_type", &type, > + sizeof type, 0) && > + ! grub_ieee1275_get_property (stdout_phandle, "name", &name, > + sizeof name, 0)) > + { > + /* > + * In general type "serial" is used for console without > + * framebuffer support in recent firmware versions then > + * we need to check the name too to determine is it real or > + * serial console > + */ > + > + if (! grub_strcmp (type, "serial")) > + { > + /* If "name" is something else than "display" we assume serial console */ > + if(! grub_strcmp (name, "display")) > + grub_ofconsole_serial = 0; > + } > + else > + { > + grub_ofconsole_serial = 0; > + > + /* Older versions use name/type set to "bootconsole" */ > + if ( grub_strcmp (name, "bootconsole")) > + grub_ofconsole_fb = 1; > + } > + } > + } Nice. On which firmware variants did you try this? I can try efika (stock firmware) if you haven't yet. In your code there's a condition in which _serial is set to 0 and _fb is left unset (as 0). Is this intended? Sounds like a bug. This could be avoided if you represent it as a multi-value variable, e.g. enum { GRUB_OFCONSOLE_SERIAL, GRUB_OFCONSOLE_FB, }; grub_u8_t grub_ofconsole_backend = GRUB_OFCONSOLE_SERIAL; [...] if (foo) grub_ofconsole_backend = GRUB_OFCONSOLE_FB; what do you think? -- Robert Millan I know my rights; I want my phone call! What use is a phone call, if you are unable to speak? (as seen on /.)