[PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag - Kernel

This is a discussion on [PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag - Kernel ; * Check for IDE_HFLAG_NO_SET_MODE host flag in ide_set_pio(), ide_set_[pio,dma]_mode(), ide_set_xfer_rate() and set_pio_mode(). * Remove no longer needed IDE_HFLAG_NO_SET_MODE host flag checking from ide_tune_dma(). * Remove superfluous ->set_pio_mode checking from do_special(). This is a part of preparations for adding 'struct ide_port_ops'. ...

+ Reply to Thread
Results 1 to 3 of 3

Thread: [PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag

  1. [PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag

    * Check for IDE_HFLAG_NO_SET_MODE host flag in ide_set_pio(),
    ide_set_[pio,dma]_mode(), ide_set_xfer_rate() and set_pio_mode().

    * Remove no longer needed IDE_HFLAG_NO_SET_MODE host flag checking
    from ide_tune_dma().

    * Remove superfluous ->set_pio_mode checking from do_special().

    This is a part of preparations for adding 'struct ide_port_ops'.

    Signed-off-by: Bartlomiej Zolnierkiewicz
    ---
    drivers/ide/ide-dma.c | 3 ---
    drivers/ide/ide-io.c | 4 ----
    drivers/ide/ide-lib.c | 12 ++++++++++--
    drivers/ide/ide.c | 4 +++-
    4 files changed, 13 insertions(+), 10 deletions(-)

    Index: b/drivers/ide/ide-dma.c
    ================================================== =================
    --- a/drivers/ide/ide-dma.c
    +++ b/drivers/ide/ide-dma.c
    @@ -706,9 +706,6 @@ static int ide_tune_dma(ide_drive_t *dri
    if (!speed)
    return 0;

    - if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    - return 1;
    -
    if (ide_set_dma_mode(drive, speed))
    return 0;

    Index: b/drivers/ide/ide-io.c
    ================================================== =================
    --- a/drivers/ide/ide-io.c
    +++ b/drivers/ide/ide-io.c
    @@ -726,10 +726,6 @@ static ide_startstop_t do_special (ide_d
    s->b.set_tune = 0;

    if (set_pio_mode_abuse(drive->hwif, req_pio)) {
    -
    - if (hwif->set_pio_mode == NULL)
    - return ide_stopped;
    -
    /*
    * take ide_lock for drive->[no_]unmask/[no_]io_32bit
    */
    Index: b/drivers/ide/ide-lib.c
    ================================================== =================
    --- a/drivers/ide/ide-lib.c
    +++ b/drivers/ide/ide-lib.c
    @@ -300,7 +300,8 @@ void ide_set_pio(ide_drive_t *drive, u8
    ide_hwif_t *hwif = drive->hwif;
    u8 host_pio, pio;

    - if (hwif->set_pio_mode == NULL)
    + if (hwif->set_pio_mode == NULL ||
    + (hwif->host_flags & IDE_HFLAG_NO_SET_MODE))
    return;

    BUG_ON(hwif->pio_mask == 0x00);
    @@ -353,6 +354,9 @@ int ide_set_pio_mode(ide_drive_t *drive,
    {
    ide_hwif_t *hwif = drive->hwif;

    + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    + return 0;
    +
    if (hwif->set_pio_mode == NULL)
    return -1;

    @@ -380,6 +384,9 @@ int ide_set_dma_mode(ide_drive_t *drive,
    {
    ide_hwif_t *hwif = drive->hwif;

    + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    + return 0;
    +
    if (hwif->set_dma_mode == NULL)
    return -1;

    @@ -410,7 +417,8 @@ int ide_set_xfer_rate(ide_drive_t *drive
    {
    ide_hwif_t *hwif = drive->hwif;

    - if (hwif->set_dma_mode == NULL)
    + if (hwif->set_dma_mode == NULL ||
    + (hwif->host_flags & IDE_HFLAG_NO_SET_MODE))
    return -1;

    rate = ide_rate_filter(drive, rate);
    Index: b/drivers/ide/ide.c
    ================================================== =================
    --- a/drivers/ide/ide.c
    +++ b/drivers/ide/ide.c
    @@ -586,11 +586,13 @@ out:
    int set_pio_mode(ide_drive_t *drive, int arg)
    {
    struct request rq;
    + ide_hwif_t *hwif = drive->hwif;

    if (arg < 0 || arg > 255)
    return -EINVAL;

    - if (drive->hwif->set_pio_mode == NULL)
    + if (hwif->set_pio_mode == NULL ||
    + (hwif->host_flags & IDE_HFLAG_NO_SET_MODE))
    return -ENOSYS;

    if (drive->special.b.set_tune)
    --
    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/

  2. Re: [PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag

    Hello.

    Bartlomiej Zolnierkiewicz wrote:

    > * Check for IDE_HFLAG_NO_SET_MODE host flag in ide_set_pio(),
    > ide_set_[pio,dma]_mode(), ide_set_xfer_rate() and set_pio_mode().


    > * Remove no longer needed IDE_HFLAG_NO_SET_MODE host flag checking
    > from ide_tune_dma().


    > * Remove superfluous ->set_pio_mode checking from do_special().


    > This is a part of preparations for adding 'struct ide_port_ops'.


    > Signed-off-by: Bartlomiej Zolnierkiewicz


    > Index: b/drivers/ide/ide-lib.c
    > ================================================== =================
    > --- a/drivers/ide/ide-lib.c
    > +++ b/drivers/ide/ide-lib.c
    > @@ -353,6 +354,9 @@ int ide_set_pio_mode(ide_drive_t *drive,
    > {
    > ide_hwif_t *hwif = drive->hwif;
    >
    > + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    > + return 0;
    > +


    Shouldn't this be considered an error?

    > if (hwif->set_pio_mode == NULL)
    > return -1;
    >
    > @@ -380,6 +384,9 @@ int ide_set_dma_mode(ide_drive_t *drive,
    > {
    > ide_hwif_t *hwif = drive->hwif;
    >
    > + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    > + return 0;
    > +


    Same here....

    > if (hwif->set_dma_mode == NULL)
    > return -1;
    >
    > @@ -410,7 +417,8 @@ int ide_set_xfer_rate(ide_drive_t *drive
    > {
    > ide_hwif_t *hwif = drive->hwif;
    >
    > - if (hwif->set_dma_mode == NULL)
    > + if (hwif->set_dma_mode == NULL ||
    > + (hwif->host_flags & IDE_HFLAG_NO_SET_MODE))
    > return -1;


    Hm, this is was not considered an error before...

    MBR, Sergei
    --
    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/

  3. Re: [PATCH 3/4] ide: sanitize handling of IDE_HFLAG_NO_SET_MODE host flag


    Hi,

    On Wednesday 12 March 2008, Sergei Shtylyov wrote:
    > Hello.
    >
    > Bartlomiej Zolnierkiewicz wrote:
    >
    > > * Check for IDE_HFLAG_NO_SET_MODE host flag in ide_set_pio(),
    > > ide_set_[pio,dma]_mode(), ide_set_xfer_rate() and set_pio_mode().

    >
    > > * Remove no longer needed IDE_HFLAG_NO_SET_MODE host flag checking
    > > from ide_tune_dma().

    >
    > > * Remove superfluous ->set_pio_mode checking from do_special().

    >
    > > This is a part of preparations for adding 'struct ide_port_ops'.

    >
    > > Signed-off-by: Bartlomiej Zolnierkiewicz

    >
    > > Index: b/drivers/ide/ide-lib.c
    > > ================================================== =================
    > > --- a/drivers/ide/ide-lib.c
    > > +++ b/drivers/ide/ide-lib.c
    > > @@ -353,6 +354,9 @@ int ide_set_pio_mode(ide_drive_t *drive,
    > > {
    > > ide_hwif_t *hwif = drive->hwif;
    > >
    > > + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    > > + return 0;
    > > +

    >
    > Shouldn't this be considered an error?


    Nope, we want to cheat kernel-level PIO tuning into succeeding.

    [ This host flag is used by it821x in "smart" mode so HW takes care of
    tuning transfer mode on controller / device itself. ]

    > > if (hwif->set_pio_mode == NULL)
    > > return -1;
    > >
    > > @@ -380,6 +384,9 @@ int ide_set_dma_mode(ide_drive_t *drive,
    > > {
    > > ide_hwif_t *hwif = drive->hwif;
    > >
    > > + if (hwif->host_flags & IDE_HFLAG_NO_SET_MODE)
    > > + return 0;
    > > +

    >
    > Same here....


    same for DMA tuning

    > > if (hwif->set_dma_mode == NULL)
    > > return -1;
    > >
    > > @@ -410,7 +417,8 @@ int ide_set_xfer_rate(ide_drive_t *drive
    > > {
    > > ide_hwif_t *hwif = drive->hwif;
    > >
    > > - if (hwif->set_dma_mode == NULL)
    > > + if (hwif->set_dma_mode == NULL ||
    > > + (hwif->host_flags & IDE_HFLAG_NO_SET_MODE))
    > > return -1;

    >
    > Hm, this is was not considered an error before...


    Yep, but transfer mode changes are unsupported on it821x in "smart" mode.

    PS Thanks for detailed review of 'struct ide_dma_ops' patch
    (I'll recast the patch as soon as possible, hopefully tomorrow).

    Thanks,
    Bart
    --
    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/

+ Reply to Thread