lkml.org 
[lkml]   [2005]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [rfc][patch] ide: fix unneeded LBA48 taskfile registers access
    Date
    On Monday 07 February 2005 05:47, Tejun Heo wrote:
    > Hello, Bartlomiej.
    >
    > Bartlomiej Zolnierkiewicz wrote:
    > > [ against ide-dev-2.6 tree, boot tested on LBA48 drive ]
    > >
    > > This small patch fixes unneeded writes/reads to LBA48 taskfile registers
    > > on LBA48 capable disks for following cases:
    > >
    > > * Power Management requests
    > > (WIN_FLUSH_CACHE[_EXT], WIN_STANDBYNOW1, WIN_IDLEIMMEDIATE commands)
    > > * special commands (WIN_SPECIFY, WIN_RESTORE, WIN_SETMULT)
    > > * Host Protected Area support (WIN_READ_NATIVE_MAX, WIN_SET_MAX)
    > > * /proc/ide/ SMART support (WIN_SMART with SMART_ENABLE,
    > > SMART_READ_VALUES and SMART_READ_THRESHOLDS subcommands)
    > > * write cache enabling/disabling in ide-disk
    > > (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_WCACHE)
    > > * write cache flushing in ide-disk (WIN_FLUSH_CACHE[_EXT])
    > > * acoustic management in ide-disk
    > > (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_AAM)
    > > * door (un)locking in ide-disk (WIN_DOORLOCK, WIN_DOORUNLOCK)
    > > * /proc/ide/hd?/identify support (WIN_IDENTIFY)
    > >
    > > Patch adds 'unsinged long flags' to ide_task_t and uses ATA_TFLAG_LBA48
    > > flag (from <linux/ata.h>) to indicate need of accessing LBA48 taskfile
    > > registers.
    >
    > ide_task_t already has fields to enable/disable writing/reading of
    > taskfile and hob registers (tf_in_flags and tf_out_flags). How about
    > adding the following two helpers.
    >
    > static inline ide_init_task[_std](ide_task_t *task)
    > {
    > memset(task, 0, sizeof(*task));
    > task->tf_out_flags = IDE_TASKFILE_STD_OUT_FLAGS;
    > task->tf_in_flags = IDE_TASKFILE_STD_IN_FLAGS;
    > }
    >
    > static inline ide_init_task_lba48(ide_task_t *task)
    > {
    > memset(task, 0, sizeof(*task));
    > task->tf_out_flags = IDE_TASKFILE_STD_OUT_FLAGS
    > | (IDE_HOB_STD_OUT_FLAGS << 8);
    > task->tf_in_flags = IDE_TASKFILE_STD_IN_FLAGS
    > | (IDE_HOB_STD_IN_FLAGS << 8);
    > }
    >
    > And, when writing taskfile,
    >
    > if (task->tf_out_flags & 0xf == IDE_TASKFILE_STD_OUT_FLAGS) {
    > /* Write taskfile regs without testing flags */
    > } else {
    > /* Flagged taskfile */
    > }
    >
    > if (task->tf_out_flags & 0xf0 == IDE_TASKFILE_HOB_OUT_FLAGS) {
    > /* Write hob regs without testing flags */
    > } else {
    > /* Flagged hob */
    > }
    >
    > And, when reading taskfile back,
    >
    > if (task->tf_in_flags & 0xf0) {
    > /* Read all hob regs */
    > }
    >
    > /* Read all taskfile regs */
    >
    > And, we need to check if any of in/out flags is zero in ioctl
    > functions and set STD flags appropriately. So, basically, in kernel,
    > tf_{in|out}_flags == 0 now means 0, not default.
    >
    > By doing like above,
    >
    > 1. Virtually the same effect
    > 2. No need to add new field to ide_task_t
    > 3. Cleaner semantics (ATA_TFLAG_LBA48 and tf_{in|out}_flags carry
    > duplicate information.)

    I would prefer to not teach do_rw_taskfile() about ->tf_{in,out}_flags
    (and convert all users to use helpers) - it is much simpler this way,

    ->flags field in ide_task_t is needed anyway (32-bit I/O flag).

    > Thanks.
    >
    > --
    > tejun
    >
    >
    -
    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 14:10    [W:0.053 / U:57.884 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site