All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Alan Cox <alan@linux.intel.com>,
	linux-ide@vger.kernel.org, jeff@garzik.org
Subject: Re: [PATCH RFC] ata: Intel IDE-R support
Date: Tue, 17 Aug 2010 18:59:38 +0200	[thread overview]
Message-ID: <4C6ABFFA.2000502@kernel.org> (raw)
In-Reply-To: <20100817180158.179e7780@lxorguk.ukuu.org.uk>

Hello, Alan.

On 08/17/2010 07:01 PM, Alan Cox wrote:
>> ata_piix already has list of all the devices it supports, so maybe
>> it's safe to grab all intel IDE devices from ata_generic?  ata_piix
> 
> I can't guarantee that and I'm not sure I can find anyone at Intel with
> that deep a knowledge of the earliest chip errata.

But ata_generic grabbing a controller doesn't mean it's gonna be
messed up in anyway.  That's what windows does for unknown IDE
controllers after all.  The problem we want to avoid here is using
ata_generic for a controller which already has a proper driver.

>> always has higher priority than ata_generic anyway and if a device
>> isn't grabbed by ata_piix, we don't lose anything by grabbing it from
>> ata_generic.
> 
> Priority only works if you know what is there to use, and the piix one is
> for example broken if you change the bindings with sysfs. You don't for
> example want to forget to compile in one driver and get generic when that
> is unsafe.

I don't really think it would be dangerous to grab intel IDE
controllers with ata_generic.  Again, that's what windows would do.
And for sysfs unbind case, if the user specifically unbinds the
controller from ata_piix, what is broken?

> The extreme example would be if you have ata_generic binding to all
> devices and you forgot to compile in RZ1000 support. On the relevant
> board that just ate your file system.
> 
> So the grab it all approach was dismissed.

For RZ1000, sure, but we're talking about only intel IDEs here.

>>> Is it documented ? The instructions it was written to came from the
>>> people who do the chips.
>>
>> Yeap, that's much better, but I still think it would be better to
>> avoid such detection magics if possible.
> 
> Well yes but the logic is simple.
> 
> 0xF8 is non zero on all later Intel ATA PCI chipsets
> 0x40 is writable on all earlier Intel ATA PCI chipsets, and zero + non
> writable on the IDE-R devices.

Maybe it's okay now but who's gonna remember what's going on there
after five years and nobody guarantees the above would continue to
hold in the future.

> The other thing I did consider was submitting an Intel IDE-R driver that
> contained the check and matched IDE CLASS & vendor == INTEL. That has the
> advantage of not getting autoloaded unnecessarily so easily, but means we
> have another driver.

Yeah, I think it would be better to do it in ata_generic one way or
the other.

Thanks.

-- 
tejun

  reply	other threads:[~2010-08-17 17:03 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-10 15:56 [PATCH RFC] ata: Intel IDE-R support Alan Cox
2010-08-10 17:12 ` Sergei Shtylyov
2010-08-10 22:23   ` Alan Cox
2010-08-17 16:19 ` Tejun Heo
2010-08-17 16:42   ` Alan Cox
2010-08-17 16:30     ` Tejun Heo
2010-08-17 17:01       ` Alan Cox
2010-08-17 16:59         ` Tejun Heo [this message]
2010-08-17 18:23           ` Alan Cox
2010-08-18  6:19             ` Tejun Heo
2010-08-18 10:03               ` Alan Cox
2010-08-18 14:10                 ` Tejun Heo
2010-08-18 15:15                   ` Alan Cox
2010-08-19  9:37                     ` Tejun Heo
2010-08-19 10:09                       ` Alan Cox
2010-08-19 11:22                         ` Tejun Heo
2010-08-19 11:35                           ` Kay Sievers
2010-08-19 11:42                             ` Tejun Heo
2010-08-19 12:24                               ` Kay Sievers
2010-08-19 12:33                                 ` Tejun Heo
2010-08-19 12:52                                   ` Kay Sievers
2010-08-19 12:54                                     ` Tejun Heo
2010-08-19 13:08                                       ` Kay Sievers
2010-08-19 13:14                                         ` Tejun Heo
2010-08-19 12:56                           ` Tejun Heo
2010-08-19 18:05                             ` Jeff Garzik
2010-08-19 11:02                       ` Tim Small

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=4C6ABFFA.2000502@kernel.org \
    --to=tj@kernel.org \
    --cc=alan@linux.intel.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    /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.