lkml.org 
[lkml]   [2002]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: ide cleanup
    On Wed, Feb 06, 2002 at 07:34:01PM -0800, Andre Hedrick wrote:

    > BZZT, I don't think so.
    >
    > Since you have not the input or insight to why it was expanded maybe you
    > should just leave it alone. Since I am working on how to fix the
    > MUNGE/KLUDGE that was added to make the PIO work, just maybe you may want
    > rething the rathole you just created by attempting to compress the code
    > before it is finished. What you doing is BORKING the driver to the point
    > where adding in the TCQ will not work. Next you have screwed the future
    > interface for Serial ATA.
    >
    > Have a good day now!

    Ok, Andre, how about the other cleanup bits of the patch? I think those
    don't "BORK" anything?

    > Andre Hedrick
    > Linux Disk Certification Project Linux ATA Development
    >
    >
    > On Wed, 6 Feb 2002, Pavel Machek wrote:
    >
    > > Hi!
    > >
    > > IDE is
    > > * infested with polish notation
    > > * programmed by cut-copy-paste
    > > * full of unneccessary casts
    > > (there are examples of all these in the patch)
    > > -> totally unreadable
    > >
    > > This attempts to clean worst mess in ide-disk.c. Dave, please apply;
    > > or if you feel it is "too big" let me know and I'll feed linus.
    > > Pavel
    > >
    > > --- clean/drivers/ide/ide-disk.c Thu Jan 31 23:42:14 2002
    > > +++ linux-dm/drivers/ide/ide-disk.c Wed Feb 6 21:43:51 2002
    > > @@ -123,29 +123,24 @@
    > > */
    > > static ide_startstop_t do_rw_disk (ide_drive_t *drive, struct request *rq, unsigned long block)
    > > {
    > > - if (rq->flags & REQ_CMD)
    > > - goto good_command;
    > > -
    > > - blk_dump_rq_flags(rq, "do_rw_disk, bad command");
    > > - ide_end_request(0, HWGROUP(drive));
    > > - return ide_stopped;
    > > -
    > > -good_command:
    > > + if (!(rq->flags & REQ_CMD)) {
    > > + blk_dump_rq_flags(rq, "do_rw_disk, bad command");
    > > + ide_end_request(0, HWGROUP(drive));
    > > + return ide_stopped;
    > > + }
    > >
    > > -#ifdef CONFIG_BLK_DEV_PDC4030
    > > if (IS_PDC4030_DRIVE) {
    > > extern ide_startstop_t promise_rw_disk(ide_drive_t *, struct request *, unsigned long);
    > > return promise_rw_disk(drive, rq, block);
    > > }
    > > -#endif /* CONFIG_BLK_DEV_PDC4030 */
    > >
    > > if ((drive->id->cfs_enable_2 & 0x0400) && (drive->addressing)) /* 48-bit LBA */
    > > - return lba_48_rw_disk(drive, rq, (unsigned long long) block);
    > > + return lba_48_rw_disk(drive, rq, block);
    > > if (drive->select.b.lba) /* 28-bit LBA */
    > > - return lba_28_rw_disk(drive, rq, (unsigned long) block);
    > > + return lba_28_rw_disk(drive, rq, block);
    > >
    > > /* 28-bit CHS : DIE DIE DIE piece of legacy crap!!! */
    > > - return chs_rw_disk(drive, rq, (unsigned long) block);
    > > + return chs_rw_disk(drive, rq, block);
    > > }
    > >
    > > static task_ioreg_t get_command (ide_drive_t *drive, int cmd)
    > > @@ -172,6 +167,30 @@
    > > return WIN_NOP;
    > > }
    > >
    > > +static int get_sectors (ide_drive_t *drive, struct request *rq, task_ioreg_t command)
    > > +{
    > > + int sectors = rq->nr_sectors;
    > > +
    > > + if (sectors == 256)
    > > + sectors = 0;
    > > + if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
    > > + sectors = drive->mult_count;
    > > + if (sectors > rq->current_nr_sectors)
    > > + sectors = rq->current_nr_sectors;
    > > + }
    > > + return sectors;
    > > +}
    > > +
    > > +static void fill_args (ide_task_t *args, struct hd_drive_task_hdr *taskfile, struct hd_drive_hob_hdr *hobfile)
    > > +{
    > > + memcpy(args->tfRegister, taskfile, sizeof(struct hd_drive_task_hdr));
    > > + memcpy(args->hobRegister, hobfile, sizeof(struct hd_drive_hob_hdr));
    > > + args->command_type = ide_cmd_type_parser(args);
    > > + args->prehandler = ide_pre_handler_parser(taskfile, hobfile);
    > > + args->handler = ide_handler_parser(taskfile, hobfile);
    > > + args->posthandler = NULL;
    > > +}
    > > +
    > > static ide_startstop_t chs_rw_disk (ide_drive_t *drive, struct request *rq, unsigned long block)
    > > {
    > > struct hd_drive_task_hdr taskfile;
    > > @@ -186,17 +205,10 @@
    > > unsigned int head = (track % drive->head);
    > > unsigned int cyl = (track / drive->head);
    > >
    > > - memset(&taskfile, 0, sizeof(task_struct_t));
    > > - memset(&hobfile, 0, sizeof(hob_struct_t));
    > > + memset(&taskfile, 0, sizeof(taskfile));
    > > + memset(&hobfile, 0, sizeof(hobfile));
    > >
    > > - sectors = rq->nr_sectors;
    > > - if (sectors == 256)
    > > - sectors = 0;
    > > - if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
    > > - sectors = drive->mult_count;
    > > - if (sectors > rq->current_nr_sectors)
    > > - sectors = rq->current_nr_sectors;
    > > - }
    > > + sectors = get_sectors(drive, rq, command);
    > >
    > > taskfile.sector_count = sectors;
    > > taskfile.sector_number = sect;
    > > @@ -215,16 +227,10 @@
    > > printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
    > > #endif
    > >
    > > - memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
    > > - memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
    > > - args.command_type = ide_cmd_type_parser(&args);
    > > - args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
    > > - args.handler = ide_handler_parser(&taskfile, &hobfile);
    > > - args.posthandler = NULL;
    > > - args.rq = (struct request *) rq;
    > > + fill_args(&args, &taskfile, &hobfile);
    > > + args.rq = rq;
    > > args.block = block;
    > > - rq->special = NULL;
    > > - rq->special = (ide_task_t *)&args;
    > > + rq->special = &args;
    > >
    > > return do_rw_taskfile(drive, &args);
    > > }
    > > @@ -237,18 +243,10 @@
    > > int sectors;
    > >
    > > task_ioreg_t command = get_command(drive, rq_data_dir(rq));
    > > + sectors = get_sectors(drive, rq, command);
    > >
    > > - sectors = rq->nr_sectors;
    > > - if (sectors == 256)
    > > - sectors = 0;
    > > - if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
    > > - sectors = drive->mult_count;
    > > - if (sectors > rq->current_nr_sectors)
    > > - sectors = rq->current_nr_sectors;
    > > - }
    > > -
    > > - memset(&taskfile, 0, sizeof(task_struct_t));
    > > - memset(&hobfile, 0, sizeof(hob_struct_t));
    > > + memset(&taskfile, 0, sizeof(taskfile));
    > > + memset(&hobfile, 0, sizeof(hobfile));
    > >
    > > taskfile.sector_count = sectors;
    > > taskfile.sector_number = block;
    > > @@ -267,16 +265,10 @@
    > > printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
    > > #endif
    > >
    > > - memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
    > > - memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
    > > - args.command_type = ide_cmd_type_parser(&args);
    > > - args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
    > > - args.handler = ide_handler_parser(&taskfile, &hobfile);
    > > - args.posthandler = NULL;
    > > - args.rq = (struct request *) rq;
    > > + fill_args(&args, &taskfile, &hobfile);
    > > + args.rq = rq;
    > > args.block = block;
    > > - rq->special = NULL;
    > > - rq->special = (ide_task_t *)&args;
    > > + rq->special = &args;
    > >
    > > return do_rw_taskfile(drive, &args);
    > > }
    > > @@ -296,17 +288,10 @@
    > >
    > > task_ioreg_t command = get_command(drive, rq_data_dir(rq));
    > >
    > > - memset(&taskfile, 0, sizeof(task_struct_t));
    > > - memset(&hobfile, 0, sizeof(hob_struct_t));
    > > + memset(&taskfile, 0, sizeof(taskfile));
    > > + memset(&hobfile, 0, sizeof(hobfile));
    > >
    > > - sectors = rq->nr_sectors;
    > > - if (sectors == 256)
    > > - sectors = 0;
    > > - if (command == WIN_MULTWRITE_EXT || command == WIN_MULTWRITE) {
    > > - sectors = drive->mult_count;
    > > - if (sectors > rq->current_nr_sectors)
    > > - sectors = rq->current_nr_sectors;
    > > - }
    > > + sectors = get_sectors(drive, rq, command);
    > >
    > > taskfile.sector_count = sectors;
    > > hobfile.sector_count = sectors >> 8;
    > > @@ -336,16 +321,10 @@
    > > printk("buffer=0x%08lx\n", (unsigned long) rq->buffer);
    > > #endif
    > >
    > > - memcpy(args.tfRegister, &taskfile, sizeof(struct hd_drive_task_hdr));
    > > - memcpy(args.hobRegister, &hobfile, sizeof(struct hd_drive_hob_hdr));
    > > - args.command_type = ide_cmd_type_parser(&args);
    > > - args.prehandler = ide_pre_handler_parser(&taskfile, &hobfile);
    > > - args.handler = ide_handler_parser(&taskfile, &hobfile);
    > > - args.posthandler = NULL;
    > > - args.rq = (struct request *) rq;
    > > + fill_args(&args, &taskfile, &hobfile);
    > > + args.rq = rq;
    > > args.block = block;
    > > - rq->special = NULL;
    > > - rq->special = (ide_task_t *)&args;
    > > + rq->special = &args;
    > >
    > > return do_rw_taskfile(drive, &args);
    > > }
    > > --- clean/drivers/ide/ide-proc.c Thu Jan 31 23:42:14 2002
    > > +++ linux-dm/drivers/ide/ide-proc.c Mon Feb 4 23:05:09 2002
    > > @@ -591,13 +591,13 @@
    > > (char *page, char **start, off_t off, int count, int *eof, void *data)
    > > {
    > > ide_drive_t *drive = (ide_drive_t *) data;
    > > - ide_driver_t *driver = (ide_driver_t *) drive->driver;
    > > + ide_driver_t *driver = drive->driver;
    > > int len;
    > >
    > > if (!driver)
    > > len = sprintf(page, "(none)\n");
    > > else
    > > - len = sprintf(page,"%llu\n", (__u64) ((ide_driver_t *)drive->driver)->capacity(drive));
    > > + len = sprintf(page,"%llu\n", (__u64) drive->driver->capacity(drive));
    > > PROC_IDE_READ_RETURN(page,start,off,count,eof,len);
    > > }
    > >
    > > @@ -629,7 +629,7 @@
    > > (char *page, char **start, off_t off, int count, int *eof, void *data)
    > > {
    > > ide_drive_t *drive = (ide_drive_t *) data;
    > > - ide_driver_t *driver = (ide_driver_t *) drive->driver;
    > > + ide_driver_t *driver = drive->driver;
    > > int len;
    > >
    > > if (!driver)
    > > @@ -746,7 +746,6 @@
    > > struct proc_dir_entry *ent;
    > > struct proc_dir_entry *parent = hwif->proc;
    > > char name[64];
    > > -// ide_driver_t *driver = drive->driver;
    > >
    > > if (drive->present && !drive->proc) {
    > > drive->proc = proc_mkdir(drive->name, parent);
    > > --- clean/include/linux/ide.h Thu Jan 31 23:42:29 2002
    > > +++ linux-dm/include/linux/ide.h Wed Feb 6 21:32:41 2002
    > > @@ -424,12 +425,12 @@
    > > unsigned long capacity; /* total number of sectors */
    > > unsigned long long capacity48; /* total number of sectors */
    > > unsigned int drive_data; /* for use by tuneproc/selectproc as needed */
    > > - void *hwif; /* actually (ide_hwif_t *) */
    > > + struct hwif_s *hwif; /* actually (ide_hwif_t *) */
    > > wait_queue_head_t wqueue; /* used to wait for drive in open() */
    > > struct hd_driveid *id; /* drive model identification info */
    > > struct hd_struct *part; /* drive partition table */
    > > char name[4]; /* drive name, such as "hda" */
    > > - void *driver; /* (ide_driver_t *) */
    > > + struct ide_driver_s *driver; /* (ide_driver_t *) */
    > > void *driver_data; /* extra driver data */
    > > devfs_handle_t de; /* directory for device */
    > > struct proc_dir_entry *proc; /* /proc/ide/ directory entry */
    > >
    > >
    > > --
    > > (about SSSCA) "I don't say this lightly. However, I really think that the U.S.
    > > no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
    >

    --
    Vojtech Pavlik
    SuSE Labs
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 13:24    [W:4.273 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site