All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Shane McDonald <mcdonald.shane@gmail.com>,
	Sergei Shtylyov <sshtylyov@ru.mvista.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Resurrect IT8172 IDE controller driver
Date: Thu, 4 Dec 2008 20:37:21 +0100	[thread overview]
Message-ID: <200812042037.21323.bzolnier@gmail.com> (raw)
In-Reply-To: <20081204140742.1d0d8046@lxorguk.ukuu.org.uk>

On Thursday 04 December 2008, Alan Cox wrote:
> On Thu, 4 Dec 2008 08:01:49 -0600
> "Shane McDonald" <mcdonald.shane@gmail.com> wrote:
> 
> > Would it be acceptable to claim that these changes are outside the
> > scope of this patch, and code the IT8172 driver to behave in the same
> > manner as the other wrong drivers, with a suitable comment indicating
> > this fact?
> 
> I think so..  and if its moved over to libata it can just be fixed then

It shouldn't be necessary...

Shane, you can use helpers from <linux/ata.h> just fine in IDE host drivers
so looking at what libata-core helper used by ata_piix.c:

        if (ata_pio_need_iordy(adev))
                control |= 2;   /* IE enable */

is actually doing:

unsigned int ata_pio_need_iordy(const struct ata_device *adev)
{
        /* Controller doesn't support  IORDY. Probably a pointless check
           as the caller should know this */
        if (adev->link->ap->flags & ATA_FLAG_NO_IORDY)
                return 0;
        /* PIO3 and higher it is mandatory */
        if (adev->pio_mode > XFER_PIO_2)
                return 1;
        /* We turn it on when possible */
        if (ata_id_has_iordy(adev->id))
                return 1;
        return 0;
}

we see that all you need to add in your driver is an additional checking
for ata_id_has_iordy().

Thanks,
Bart

PS when it comes to actually setting the transfer mode on the device
(done in ide_config_drive_speed() core function) IORDY is also handled
just fine (thanks to Sergei).

  reply	other threads:[~2008-12-04 19:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-24  6:21 [PATCH] Resurrect IT8172 IDE controller driver Shane McDonald
2008-11-24  9:55 ` Alan Cox
2008-11-24 10:02   ` Sergei Shtylyov
2008-11-24 10:33 ` Sergei Shtylyov
2008-11-24 12:12   ` Sergei Shtylyov
2008-11-24 12:32     ` Alan Cox
2008-11-24 13:38       ` Sergei Shtylyov
2008-12-04  3:08         ` Shane McDonald
2008-12-04 16:10           ` Sergei Shtylyov
2008-12-04  2:39   ` Shane McDonald
2008-12-04 10:07     ` Alan Cox
2008-12-04 14:01       ` Shane McDonald
2008-12-04 14:07         ` Alan Cox
2008-12-04 19:37           ` Bartlomiej Zolnierkiewicz [this message]
2008-12-04 16:17       ` Sergei Shtylyov
2008-12-08 12:02     ` Sergei Shtylyov
2008-12-22  7:50       ` Shane McDonald

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200812042037.21323.bzolnier@gmail.com \
    --to=bzolnier@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcdonald.shane@gmail.com \
    --cc=sshtylyov@ru.mvista.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.