From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1KEjfs-0004wK-Qu for mharc-grub-devel@gnu.org; Fri, 04 Jul 2008 07:42:00 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KEjfr-0004w4-Uy for grub-devel@gnu.org; Fri, 04 Jul 2008 07:42:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KEjfq-0004vJ-4G for grub-devel@gnu.org; Fri, 04 Jul 2008 07:41:59 -0400 Received: from [199.232.76.173] (port=36801 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KEjfp-0004vA-Rs for grub-devel@gnu.org; Fri, 04 Jul 2008 07:41:57 -0400 Received: from smtp-vbr17.xs4all.nl ([194.109.24.37]:3533) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KEjfo-0005xV-9f for grub-devel@gnu.org; Fri, 04 Jul 2008 07:41:57 -0400 Received: from localhost.localdomain (249-174.surfsnel.dsl.internl.net [145.99.174.249]) by smtp-vbr17.xs4all.nl (8.13.8/8.13.8) with ESMTP id m64Bfsan063514 for ; Fri, 4 Jul 2008 13:41:54 +0200 (CEST) (envelope-from mgerards@xs4all.nl) From: Marco Gerards To: The development of GRUB 2 References: <20080703001716.13881.1754.stgit@dv.roinet.com> <87lk0i6cxo.fsf@xs4all.nl> <1215118211.2734.8.camel@dv> Mail-Copies-To: mgerards@xs4all.nl Date: Fri, 04 Jul 2008 13:50:21 +0200 In-Reply-To: <1215118211.2734.8.camel@dv> (Pavel Roskin's message of "Thu, 03 Jul 2008 16:50:11 -0400") Message-ID: <871w2927ia.fsf@xs4all.nl> User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Virus-Scanned: by XS4ALL Virus Scanner X-detected-kernel: by monty-python.gnu.org: FreeBSD 4.6-4.9 Subject: Re: [PATCH] Enable writing to ATA devices, fix several bugs 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: Fri, 04 Jul 2008 11:42:00 -0000 Pavel Roskin writes: > On Thu, 2008-07-03 at 20:27 +0200, Marco Gerards wrote: > >> The more patches I get, the better ;-) >> >> > ChangeLog: >> > >> > * disk/ata.c (grub_ata_pio_write): Check status before writing, >> > like we do in grub_ata_pio_read(). >> > >> > (grub_ata_readwrite): Always write individual sectors. Fix the >> > sector count for the remainder. >> >> Why? > > Because we do it elsewhere. I assume you forgot to convert the code for > writing, but you meant to do it: > > r1335 | marco_g | 2007-11-03 08:25:19 -0400 (Sat, 03 Nov 2007) | 6 lines > > 2007-11-03 Marco Gerards > > * disk/ata.c (grub_ata_readwrite): Call grub_ata_pio_read and > grub_ata_pio_write once for every single sector, instead of for > multiple sectors. > > I guess it's safer. We can explore some optimization, but first we > should make it reliable. Ah right :-) >> > (grub_ata_write): Enable writing to ATA devices. Correctly >> > report error for ATAPI devices. > >> Great! Did you test this? > > Yes, I tested this part. env_save didn't report any error originally, > so I introduced grub_error(), and env_save started reporting the error. > Then I enabled writing and tested it in qemu. > > I cannot get the ata module to recognize the hard drive on my test > machine, so more work is needed to test it on the real hardware. Right, this needs more work. I will have another look at ATA soon :-) For me ATA support worked on real hardware. I will have to try it on more hardware. >> If you can fix Roberts comment, it would be great! Can you commit it >> afterwards? > > Sure. > > If you check Linux include/linux/ata.h, 1 is ATA_ERR, and we really need > ata_ok(), which checks multiple flags. So it's clearly material for a > separate patch. No, I cannot check the Linux code. AFAIK this can cause copyright problems. But I agree that more and better error checking is required. -- Marco