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. >> >> There are many examples where this happened. I just noticed: >> >> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt; >> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt)) >> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt; >> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt)) >> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt; >> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt)) >> commands/hdparm.c: if (! grub_disk_ata_pass_through) >> disk/ata_pthru.c: struct >> grub_disk_ata_pass_through_parms *parms) >> disk/ata_pthru.c: grub_disk_ata_pass_through = grub_ata_pass_through; >> disk/ata_pthru.c: if (grub_disk_ata_pass_through == >> grub_ata_pass_through) >> disk/ata_pthru.c: grub_disk_ata_pass_through = NULL; >> include/grub/disk.h:struct grub_disk_ata_pass_through_parms >> include/grub/disk.h:extern grub_err_t (* >> EXPORT_VAR(grub_disk_ata_pass_through)) (grub_disk_t, >> include/grub/disk.h: struct >> grub_disk_ata_pass_through_parms *); >> kern/disk.c:grub_err_t (* grub_disk_ata_pass_through) (grub_disk_t, >> 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? > 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. >
-- Regards Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel