All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-ide@vger.kernel.org, linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] ide: make ide_pci_check_iomem() actually work
Date: Tue, 08 Apr 2008 16:38:31 +0400	[thread overview]
Message-ID: <47FB6747.2050301@ru.mvista.com> (raw)
In-Reply-To: <47FB5ADF.20608@ru.mvista.com>

Hi, I just wrote:

>>> This function didn't actually check if a given BAR is in I/O space 
>>> because of
>>> using the bogus PCI_BASE_ADDRESS_IO_MASK (which equals ~3) to test 
>>> the resource
>>> flags instead of IORESOURCE_IO -- fix this, make ide_hwif_configure() 
>>> check the
>>> results failing if necessary, and move the printk() call to the 
>>> failure path.

>> This change is OK in itself but I worry that ide_pci_check_iomem() may 
>> now
>> return "false" errors (bogus PCI_BASE_ADDRESS_IO_MASK check resulted 
>> in MEM
>> resources always surviving ide_pci_check_iomem() calls before the fix) 
>> for
>> some host drivers (siimage, scc_pata...) resulting in failed 
>> initialization.

>    The SiI chips do have normal I/O resources at BAR0..BAR3. As for 
> scc_pata, the control should not even get there because BAR0..BAR3 are 
> *not* IDE command/control block bases on this chip (BAR0/1 are 
> control/DMA bases if you look into setup_mmio_scc()) but they are 
> treated as such by the code immediately following ide_pci_check_iomem() 
> calls in ide_hwif_configure(), i.e. we might have an error here. The 
> same can be said about the PowerMAC driver which has all its MMIO 
> registers at BAR0.

>> How's about removing this dead/broken function instead for now?

>    If we indeed have a MMIO problem here, it's not in this function but 
> in its callers.

     Looks like we actually have this problem with scc_pata -- it calls 
ide_setup_pci_device() which should lead to calling ide_hwif_configure(). But 
this is broken since this call chain expects a normal PCI IDE controller with 
BAR0..BAR3 either non-existant or being primary/secondary port bases in I/O space.

>> Thanks,
>> Bart

>>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

WBR, Sergei

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

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-07 16:27 [PATCH] ide: make ide_pci_check_iomem() actually work Sergei Shtylyov
2008-04-07 20:46 ` Bartlomiej Zolnierkiewicz
2008-04-08 11:45   ` Sergei Shtylyov
2008-04-08 12:38     ` Sergei Shtylyov [this message]
2008-04-09 18:34       ` Bartlomiej Zolnierkiewicz
2008-04-09 18:34         ` Bartlomiej Zolnierkiewicz
2008-04-15 20:45         ` Bartlomiej Zolnierkiewicz
2008-04-15 20:45           ` 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=47FB6747.2050301@ru.mvista.com \
    --to=sshtylyov@ru.mvista.com \
    --cc=bzolnier@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.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.