LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Sergei Shtylyov <sshtylyov@mvista.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jeff Garzik <jgarzik@pobox.com>
Subject: Re: [PATCH 05/20] pata_efar: always program master_data before slave_data
Date: Sun, 20 Feb 2011 11:38:23 +0100	[thread overview]
Message-ID: <AANLkTi=Lj54hRfkn7qLwoQ7aGSchnzBwy4m2Uok764qD@mail.gmail.com> (raw)
In-Reply-To: <20110219164811.2e90de88@lxorguk.ukuu.org.uk>

On Sat, Feb 19, 2011 at 5:48 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> >   SLC90E66 datasheet only says that SIDETIM register has no effect without
>> > SITRE bit set.
>
> Which means the current setup is fine yes ?

That's the problem, the current setup is buggy in slave-only setups:

                slave_data |= ((timings[pio][0] << 2) |
timings[pio][1]) << shift;
                pci_write_config_byte(dev, 0x44, slave_data);
        }

        idetm_data |= 0x4000;   /* Ensure SITRE is set */
        pci_write_config_word(dev, idetm_port, idetm_data);

idetm_port needs to be programmed _before_ slave_data to "ensure SITRE is set".

IOW We currently depend on the BIOS here to set SITRE bit for us..

>> Alan, is this explanation sufficient to ACK this patch?  (Thanks
>> Sergei for digging it out.)
>>
>> Jeff, would it be possible to queue patches #01-15 for 2.6.39
>
> Have they been tested on the relevant hardware yet - I don't believe the
> changes should be made for the untested stuff and you indicated you
> agreed with that.

Unless they are obvious or risk is higher than benefit (bugfixes based
on code review).  I don't think that ATA code has became recently
special in this regard compared to the rest of the kernel.

> The tidyups for the easily tested cases (PIIX4 etc) look sensible

Please take look at the changes #01-15 then:

     ata_piix: SITRE handling fix
     ata_piix: unify code for programming PIO and MWDMA timings
     pata_efar: fix register naming used in efar_set_piomode()
     pata_efar: unify code for programming PIO and MWDMA timings
     pata_efar: always program master_data before slave_data
     pata_it8213: fix register naming used in it8213_set_piomode()
     pata_it8213: unify code for programming PIO and MWDMA timings
     pata_it8213: add UDMA100 and UDMA133 support
     pata_oldpiix: unify code for programming PIO and MWDMA timings
     pata_radisys: unify code for programming PIO and MWDMA timings
     pata_rdc: unify code for programming PIO and MWDMA timings
     pata_rdc: parallel scanning needs an extra locking
     pata_rdc: add Power Management support
     pata_oldpiix: add locking for parallel scanning
     pata_oldpiix: enable parallel scan

Most patches has been posted months ago in the past as the part of
atang tree.  They don't contain risky stuff (it was intentionally
pushed at the end of the patch queue).  We have here obvious register
naming unification, PIO/DMA folding (it was tested with ata_piix and
only later ported to other PIIX-like drivers) and:

* para_rdc: locking bug-fix and Power Management support

* pata_it8213: IDE -> libata regression fix (UDMA100/133 support based
on vendor / old IDE driver)

* pata_oldpiix: locking bug-fix (ACK-ed by you in the past) and
parallel scanning support

I see a lot of magnitude more risky stuff going elsewhere in the
kernel, including ATA itself and if there are any concerns left those
patches (#01-15) I'll be happy to address them.

Thanks,
Bartlomiej

  reply	other threads:[~2011-02-20 10:38 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-08 12:23 [PATCHSET] ata: rework support for PIIX-alike PATA controllers Bartlomiej Zolnierkiewicz
2011-02-08 12:23 ` [PATCH 01/20] ata_piix: SITRE handling fix Bartlomiej Zolnierkiewicz
2011-02-08 13:04   ` Alan Cox
2011-02-08 13:02     ` Bartlomiej Zolnierkiewicz
2011-02-08 12:23 ` [PATCH 02/20] ata_piix: unify code for programming PIO and MWDMA timings Bartlomiej Zolnierkiewicz
2011-02-20 13:36   ` Sergei Shtylyov
2011-02-20 18:59     ` Alan Cox
2011-02-20 20:42       ` Sergei Shtylyov
2011-02-20 21:07         ` Alan Cox
2011-02-21 11:38           ` Sergei Shtylyov
2011-02-21 11:53             ` Sergei Shtylyov
2011-02-21 12:00               ` Alan Cox
2011-02-21 11:58             ` Alan Cox
2011-02-21 11:59               ` Sergei Shtylyov
2011-02-08 12:23 ` [PATCH 03/20] pata_efar: fix register naming used in efar_set_piomode() Bartlomiej Zolnierkiewicz
2011-02-08 12:24 ` [PATCH 04/20] pata_efar: unify code for programming PIO and MWDMA timings Bartlomiej Zolnierkiewicz
2011-02-20 14:32   ` Sergei Shtylyov
2011-02-08 12:24 ` [PATCH 05/20] pata_efar: always program master_data before slave_data Bartlomiej Zolnierkiewicz
2011-02-08 13:07   ` Alan Cox
2011-02-08 13:16     ` Bartlomiej Zolnierkiewicz
2011-02-08 13:25       ` Alan Cox
2011-02-08 13:28         ` Bartlomiej Zolnierkiewicz
2011-02-08 13:39           ` Alan Cox
2011-02-08 13:57             ` Bartlomiej Zolnierkiewicz
2011-02-08 14:12               ` Alan Cox
2011-02-08 14:32                 ` Bartlomiej Zolnierkiewicz
2011-02-08 13:38         ` Sergei Shtylyov
2011-02-19  9:25           ` Bartlomiej Zolnierkiewicz
2011-02-19 16:48             ` Alan Cox
2011-02-20 10:38               ` Bartlomiej Zolnierkiewicz [this message]
2011-02-20 11:36                 ` Alan Cox
2011-02-21 20:06             ` Jeff Garzik
2011-02-22  9:19               ` Bartlomiej Zolnierkiewicz
2011-02-22 11:14                 ` Alan Cox
2011-02-23  8:45                   ` Bartlomiej Zolnierkiewicz
2011-02-23  8:53                     ` Bartlomiej Zolnierkiewicz
2011-02-23 10:17                       ` Bartlomiej Zolnierkiewicz
2011-02-23  9:14                     ` Alan Cox
2011-02-23 10:28                       ` Bartlomiej Zolnierkiewicz
2011-02-23 10:36                         ` Alan Cox
2011-02-24 17:53                         ` Alan Cox
2011-02-10 14:23   ` Sergei Shtylyov
2011-02-10 17:14     ` Bartlomiej Zolnierkiewicz
2011-02-10 17:55       ` Sergei Shtylyov
2011-02-08 12:24 ` [PATCH 06/20] pata_it8213: fix register naming used in it8213_set_piomode() Bartlomiej Zolnierkiewicz
2011-02-08 12:24 ` [PATCH 07/20] pata_it8213: unify code for programming PIO and MWDMA timings Bartlomiej Zolnierkiewicz
2011-02-20 14:37   ` Sergei Shtylyov
2011-02-08 12:24 ` [PATCH 08/20] pata_it8213: add UDMA100 and UDMA133 support Bartlomiej Zolnierkiewicz
2011-02-08 12:24 ` [PATCH 09/20] pata_oldpiix: unify code for programming PIO and MWDMA timings Bartlomiej Zolnierkiewicz
2011-02-20 14:52   ` Sergei Shtylyov
2011-02-08 12:24 ` [PATCH 10/20] pata_radisys: " Bartlomiej Zolnierkiewicz
2011-02-20 15:01   ` Sergei Shtylyov
2011-02-08 12:24 ` [PATCH 11/20] pata_rdc: " Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 12/20] pata_rdc: parallel scanning needs an extra locking Bartlomiej Zolnierkiewicz
2011-02-20 15:04   ` Sergei Shtylyov
2011-02-08 12:25 ` [PATCH 13/20] pata_rdc: add Power Management support Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 14/20] pata_oldpiix: add locking for parallel scanning Bartlomiej Zolnierkiewicz
2011-02-10 14:28   ` Sergei Shtylyov
2011-02-08 12:25 ` [PATCH 15/20] pata_oldpiix: enable parallel scan Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 16/20] ata_piix: add EFAR SLC90E66 support Bartlomiej Zolnierkiewicz
2011-02-08 13:13   ` Alan Cox
2011-02-08 13:27     ` Bartlomiej Zolnierkiewicz
2011-02-08 13:38       ` Alan Cox
2011-02-08 14:01         ` Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 17/20] ata_piix: add IT8213 support Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 18/20] ata_piix: add RDC support Bartlomiej Zolnierkiewicz
2011-02-08 12:25 ` [PATCH 19/20] ata_piix: add Intel old PIIX support Bartlomiej Zolnierkiewicz
2011-02-08 12:26 ` [PATCH 20/20] ata_piix: add Radisys R82600 support Bartlomiej Zolnierkiewicz

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='AANLkTi=Lj54hRfkn7qLwoQ7aGSchnzBwy4m2Uok764qD@mail.gmail.com' \
    --to=bzolnier@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sshtylyov@mvista.com \
    --subject='Re: [PATCH 05/20] pata_efar: always program master_data before slave_data' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).