From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1Ne61B-0006lp-OY for mharc-grub-devel@gnu.org; Sun, 07 Feb 2010 07:13:37 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ne618-0006jp-OS for grub-devel@gnu.org; Sun, 07 Feb 2010 07:13:34 -0500 Received: from [199.232.76.173] (port=36560 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ne617-0006iz-MT for grub-devel@gnu.org; Sun, 07 Feb 2010 07:13:33 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Ne612-0001yJ-Ei for grub-devel@gnu.org; Sun, 07 Feb 2010 07:13:33 -0500 Received: from mailout09.t-online.de ([194.25.134.84]:50875) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ne611-0001xj-Rn for grub-devel@gnu.org; Sun, 07 Feb 2010 07:13:28 -0500 Received: from fwd01.aul.t-online.de (fwd01.aul.t-online.de ) by mailout09.t-online.de with smtp id 1Ne5pV-0003Bb-3c; Sun, 07 Feb 2010 13:01:33 +0100 Received: from [10.3.2.2] (GorodwZv8hMv-iT211d29cN+IySzMGy4g7qBdBPZ8TLEMfjiPrmAjXdmNTlBoLbggf@[217.235.253.93]) by fwd01.aul.t-online.de with esmtp id 1Ne5pK-15Bywi0; Sun, 7 Feb 2010 13:01:22 +0100 Message-ID: <4B6EAB93.7090102@t-online.de> Date: Sun, 07 Feb 2010 13:01:23 +0100 From: Christian Franke User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20100104 SeaMonkey/2.0.2 MIME-Version: 1.0 To: The development of GNU GRUB References: <20100125080620.GA14131@thorin> <4B5DFB57.5020703@t-online.de> <4B6E0B4F.9030209@gmail.com> In-Reply-To: <4B6E0B4F.9030209@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ID: GorodwZv8hMv-iT211d29cN+IySzMGy4g7qBdBPZ8TLEMfjiPrmAjXdmNTlBoLbggf X-TOI-MSGID: 584d4d8c-0a67-413e-8a11-d046aac18370 X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: Re: On gratuitous modularization X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Feb 2010 12:13:34 -0000 Vladimir 'φ-coder/phcoder' Serbinenko wrote: > Christian Franke wrote: > >> Hi Robert, >> >> Robert Millan wrote: >> >> >>> Please be careful when adding modules. I see that too often new modules >>> are added without any real need to host this code separately. >>> >>> ... >>> kern/disk.c: struct grub_disk_ata_pass_through_parms *); >>> >>> this seems unnecessary. ata_pthru is very small. If it's only used >>> by hdparm, >>> why not just merge it? This also avoids the additional code in kernel. >>> >>> >>> >> The module ata_pthru.mod exists only to keep ata.mod small, see: >> http://lists.gnu.org/archive/html/grub-devel/2009-02/msg00091.html >> >> Keeping the ata.mod specific pass-through function separate from >> hdparm.mod was intentional. Merging this function into hdparm.mod >> would only make sense if ata.mod will the only ATA access module with >> pass-through functionality in the future. Hdparm.mod would then depend >> on ata.mod. A 'hdparm -h' would load ata.mod and disable biosdisk access. >> >> I hope we will eventually have an ahci.mod :-) >> >> > Are the parameters of current ata_pass_through ATA-specific or would > they be the same on AHCI? > Like e.g. the Linux HDIO_DRIVE_TASKFILE ioctl, the ata_pass_through() parameters are not controller or transport specific. It should work with any possible future implementation of ATA pass-through functionality, e.g. - native AHCI driver - other native drivers - SAT (ANSI INCITS 431-2007) ATA pass through command for: -- SATA drives behind USB-bridges with SAT support, or: -- SATA drives in SAS enclosures Merging ata_pthru.mod into hdparm.mod would make hdparm specific to one (outdated) class of controllers: the traditional IDE-controller (T13/1510D). >> BTW: I agree that using a global function pointer >> 'grub_disk_ata_pass_through' is a hack. A cleaner design would be >> possible with a grub_disk_dev.ioctl(.) call. >> >> Such an ioctl() call would IMO be the best way to handle such device class specific extra functionality. -- Christian Franke