Re: [PATCH 2.6.11-rc2 0/29] ide: driver updates
On Mer, 2005-02-02 at 08:31, Jeff Garzik wrote: > > Merges drivers/ide/pci/*.h files into their corresponding *.c > > files. Rationales are > > 1. There's no reason to separate pci drivers into header and > >body. No header file is shared and they're simple enough. > > 2. struct pde_pci_device_t *_chipsets[] are _defined_ in the > >header files. That isn't the custom and there's no good > >reason to do differently in these drivers. > > 3. Tracking changelogs shows that the bugs fixed by 00 and 01 > >are introduced during mass-updating ide pci drivers by > >forgetting to update *.h files. > > Personally, I agree. However, I would ask Alan for his rationale before > applying this... Historically they were split so they stayed split. SCSI has mostly (c/o hch) switched away from that and it seems sensible for IDE to do so. > > > 07_ide_reg_valid_t_endian_fix.patch > > > > ide_reg_valid_t contains bitfield flags but doesn't reverse > > bit orders using __*_ENDIAN_BITFIELD macros. And constants > > for ide_reg_valid_t, IDE_{TASKFILE|HOB}_STD_{IN|OUT}_FLAGS, > > are defined as byte values which are correct only on > > little-endian machines. This patch defines reversed constants > > and .h byte union structure to make things correct on big > > endian machines. The only code which uses above macros is in > > flagged_taskfile() and the code is currently unused, so this > > patch doesn't change any behavior. (The code will get used in > > later patches.) > > doesn't this "fix" change behavior on existing big endian machines? My question too, remember that there is I/O byte order swizzling afoot in the I/O macros. Generally looks good IMHO. - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.11-rc2 0/29] ide: driver updates
On Wed, 2 Feb 2005 11:40:17 +0900, Tejun Heo <[EMAIL PROTECTED]> wrote: > Hello, B. Zolnierkiewicz. Hi, > These patches are various fixes/improvements to the ide driver. They > are against the 2.6 bk tree as of today (20050202). Nice series. As you probably already know I merged most of the easy stuff. I will comment on the real stuff soon. Thanks, Bartlomiej - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.11-rc2 0/29] ide: driver updates
Tejun Heo wrote: Hello, B. Zolnierkiewicz. These patches are various fixes/improvements to the ide driver. They are against the 2.6 bk tree as of today (20050202). 01_ide_remove_adma100.patch Removes drivers/ide/pci/adma100.[hc]. The driver isn't compilable (missing functions) and no Kconfig actually enables CONFIG_BLK_DEV_ADMA100. Also, the libata-dev-2.6 tree has an "ata_adma" driver which is complete, but needs some testing (and I have h/w). 05_ide_merge_pci_driver_hc.patch Merges drivers/ide/pci/*.h files into their corresponding *.c files. Rationales are 1. There's no reason to separate pci drivers into header and body. No header file is shared and they're simple enough. 2. struct pde_pci_device_t *_chipsets[] are _defined_ in the header files. That isn't the custom and there's no good reason to do differently in these drivers. 3. Tracking changelogs shows that the bugs fixed by 00 and 01 are introduced during mass-updating ide pci drivers by forgetting to update *.h files. Personally, I agree. However, I would ask Alan for his rationale before applying this... 07_ide_reg_valid_t_endian_fix.patch ide_reg_valid_t contains bitfield flags but doesn't reverse bit orders using __*_ENDIAN_BITFIELD macros. And constants for ide_reg_valid_t, IDE_{TASKFILE|HOB}_STD_{IN|OUT}_FLAGS, are defined as byte values which are correct only on little-endian machines. This patch defines reversed constants and .h byte union structure to make things correct on big endian machines. The only code which uses above macros is in flagged_taskfile() and the code is currently unused, so this patch doesn't change any behavior. (The code will get used in later patches.) doesn't this "fix" change behavior on existing big endian machines? 15_ide_flagged_taskfile_data_byte_order_fix.patch In flagged_taskfile(), when writing data register, taskfile->data goes to the lower byte and hobfile->data goes to the upper byte on little endian machines and the opposite happens on big endian machines. This patch make taskfile->data always go to the lower byte regardless of endianess. ditto 16_ide_flagged_taskfile_select_dev_bit_masking.patch In flagged_taskfile(), make off DEV bit before OR'ing it with drive->select.all when writing to IDE_SELECT_REG. Probably the right thing to do, but be very very careful you have audited all uses... 21_ide_do_taskfile.patch Merged do_rw_taskfile() and flagged_taskfile() into do_taskfile(). During the merge, the following changes took place. 1. flagged taskfile now honors HOB feature register. (do_rw_taskfile() did write to HOB feature.) 2. No do_rw_taskfile() HIHI check on select register. Except for the DEV bit, all bits are honored. 3. Uses taskfile->data_phase to determine if dma trasfer is requested. (do_rw_taskfile() directly switched on taskfile->command for all dma commands) I think Bart already had plans for this (similar to your patch)? 22_ide_taskfile_flush.patch All REQ_DRIVE_TASK users except ide_task_ioctl() converted to use REQ_DRIVE_TASKFILE. Rationale? 24_ide_remove_task.patch Unused REQ_DRIVE_TASK handling removed. this series is nice. 25_ide_taskfile_cmd.patch All in-kernel REQ_DRIVE_CMD users except for ide_cmd_ioctl() converted to use REQ_DRIVE_TASKFILE. 26_ide_taskfile_cmd_ioctl.patch ide_cmd_ioctl() converted to use ide_taskfile_ioctl(). This is the last user of REQ_DRIVE_CMD. ditto 27_ide_remove_cmd.patch Removed unused REQ_DRIVE_CMD handling. 28_ide_taskfile_init_drive_cmd.patch ide_init_drive_cmd() now initializes rq->flags to REQ_DRIVE_TASKFILE. 29_ide_explicit_TASKFILE_NO_DATA.patch Make data_phase explicit in NO_DATA cases. I would make sure this series gets some amount of testing in -mm before pushing upstream, though... Jeff - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.11-rc2 0/29] ide: driver updates
Hello, B. Zolnierkiewicz. These patches are various fixes/improvements to the ide driver. They are against the 2.6 bk tree as of today (20050202). 01_ide_remove_adma100.patch Removes drivers/ide/pci/adma100.[hc]. The driver isn't compilable (missing functions) and no Kconfig actually enables CONFIG_BLK_DEV_ADMA100. 02_ide_cleanup_it8172.patch In drivers/ide/pci/it8172.h, it8172_ratefilter() and init_setup_it8172() are declared and the latter is referenced in it8172_chipsets. Both functions are not defined or used anywhere. This patch removes the prototypes and reference. it8172 should be compilable now. 03_ide_cleanup_opti621.patch In drivers/ide/pci/opti612.[hc], init_setup_opt621() is declared, defined and referenced but never actullay used. Thie patch removes the function. 04_ide_cleanup_piix.patch In drivers/ide/pci/piix.[hc], init_setup_piix() is defined and used but only one init_setup function is defined and no demultiplexing is done using init_setup callback. As other drivers call ide_setup_pci_device() directly in such cases, this patch removes init_setup_piix() and makes piix_init_one() call ide_setup_pci_device() directly. 05_ide_merge_pci_driver_hc.patch Merges drivers/ide/pci/*.h files into their corresponding *.c files. Rationales are 1. There's no reason to separate pci drivers into header and body. No header file is shared and they're simple enough. 2. struct pde_pci_device_t *_chipsets[] are _defined_ in the header files. That isn't the custom and there's no good reason to do differently in these drivers. 3. Tracking changelogs shows that the bugs fixed by 00 and 01 are introduced during mass-updating ide pci drivers by forgetting to update *.h files. 06_ide_start_request_IDE_CONTROL_REG.patch Replaced HWIF(drive)->io_ports[IDE_CONTROL_OFFSET] with equivalent IDE_CONTROL_REG in ide-io.c. 07_ide_reg_valid_t_endian_fix.patch ide_reg_valid_t contains bitfield flags but doesn't reverse bit orders using __*_ENDIAN_BITFIELD macros. And constants for ide_reg_valid_t, IDE_{TASKFILE|HOB}_STD_{IN|OUT}_FLAGS, are defined as byte values which are correct only on little-endian machines. This patch defines reversed constants and .h byte union structure to make things correct on big endian machines. The only code which uses above macros is in flagged_taskfile() and the code is currently unused, so this patch doesn't change any behavior. (The code will get used in later patches.) 08_ide_do_identify_model_string_termination.patch Terminates id->model string before invoking strstr() in do_identify(). 09_ide_do_rw_disk_lba48_dma_check_fix.patch In __ide_do_rw_disk(), the shifted block, instead of the original rq->sector, should be used when checking range for lba48 dma. 10_ide_do_rw_disk_pre_task_out_intr_return_fix.patch In __ide_do_rw_disk(), ide_started used to be returned blindly after issusing PIO write. This can cause hang if pre_task_out_intr() returns ide_stopped due to failed ide_wait_stat() test. Fixed to pass the return value of pre_task_out_intr(). 11_ide_drive_sleeping_fix.patch ide_drive_t.sleeping field added. 0 in sleep field used to indicate inactive sleeping but because 0 is a valid jiffy value, though slim, there's a chance that something can go weird. And while at it, explicit jiffy comparisons are converted to use time_{after|before} macros. 12_ide_hwgroup_t_polling.patch ide_hwgroup_t.polling field added. 0 in poll_timeout field used to indicate inactive polling but because 0 is a valid jiffy value, though slim, there's a chance that something weird can happen. 13_ide_tape_time_after.patch Explicit jiffy comparision converted to time_after() macro. 14_ide_error_remove_NULL_test.patch In ide_error(), drive cannot be NULL. ide_dump_status() can't handle NULL drive. 15_ide_flagged_taskfile_data_byte_order_fix.patch In flagged_taskfile(), when writing data register, taskfile->data goes to the lower byte and hobfile->data goes to the upper byte on little endian machines and the opposite happens on big endian machines. This patch make taskfile->data always go to the lower byte regardless of endianess. 16_ide_flagged_taskfile_select_dev_bit_masking.patch In flagged_taskfile(), make off DEV bit before OR'ing it with drive->select.all when writing to IDE_SELECT_REG. 17_ide_flagged_taskfile_select_check.patch In flagged_taskfile(), tf_out_flags.b.select should be checked