2003-08-19 19:48:03

by Jon Smirl

[permalink] [raw]
Subject: Standard driver call to enable/disable PCI ROM

I needed to enable a PCI ROM and read a few things
from it, and then disable it again. It might be worth
adding a standard PCI API in 2.6 for this.

Here's the code I used...

static void * __init aty128_map_ROM(struct pci_dev
*dev, const struct aty128fb_par
*par)
{
void *rom;
struct resource *r =
&dev->resource[PCI_ROM_RESOURCE];

/* assign address if it doesn't have one */
if (r->start == 0)
pci_assign_resource(dev,
PCI_ROM_RESOURCE);

/* enable if needed */
if (!(r->flags & PCI_ROM_ADDRESS_ENABLE)) {
pci_write_config_dword(dev,
dev->rom_base_reg, r->start |
PCI_ROM_ADDRESS_ENABLE);
r->flags |= PCI_ROM_ADDRESS_ENABLE;
}
rom = ioremap(r->start, r->end - r->start + 1);
if (!rom) {
printk(KERN_ERR "aty128fb: ROM failed to map\n");
return NULL;
}
/* Very simple test to make sure it appeared */
if (readb(rom) != 0x55) {
printk(KERN_ERR "aty128fb: Invalid ROM
signature %x should be 0x55\n",
readb(rom));
aty128_unmap_ROM(dev, rom);
return NULL;
}
return rom;
}

static void __init aty128_unmap_ROM(struct pci_dev
*dev,
void * rom)
{
/* leave it disabled and unassigned */
struct resource *r =
&dev->resource[PCI_ROM_RESOURCE];

iounmap(rom);

r->flags &= ~PCI_ROM_ADDRESS_ENABLE;
r->end -= r->start;
r->start = 0;
/* This will disable and set address to unassigned
*/
pci_write_config_dword(dev, dev->rom_base_reg, 0);
release_resource(r);
}


=====
Jon Smirl
[email protected]

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com


2003-08-19 20:06:59

by Russell King

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

On Tue, Aug 19, 2003 at 12:45:03PM -0700, Jon Smirl wrote:
> Here's the code I used...
>
> static void * __init aty128_map_ROM(struct pci_dev
> *dev, const struct aty128fb_par
> *par)
> {
> void *rom;
> struct resource *r =
> &dev->resource[PCI_ROM_RESOURCE];
>
> /* assign address if it doesn't have one */
> if (r->start == 0)
> pci_assign_resource(dev,
> PCI_ROM_RESOURCE);
>
> /* enable if needed */
> if (!(r->flags & PCI_ROM_ADDRESS_ENABLE)) {
> pci_write_config_dword(dev,
> dev->rom_base_reg, r->start |
^^^^^^^^^

This is non-portable.

You should use pcibios_resource_to_bus() to convert a resource to a
representation suitable for a BAR.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-08-19 20:54:32

by Russell King

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

On Tue, Aug 19, 2003 at 01:46:43PM -0700, Jon Smirl wrote:
> --- Russell King <[email protected]> wrote:
> > You should use pcibios_resource_to_bus() to convert a resource to a
> > representation suitable for a BAR.
>
> I've never used pcibios_resource_to_bus(), what's the right way to do this?
> This is a good reason for making this into a common PCI driver function,
> it will stop people like me from messing up PCI calls.

It's a 2.5/2.6 invention.

Here follows a cut-down version of pci_update_resource() to illustrate
its use when updating up a BAR (see drivers/pci/setup-res.c for the full
version):

static void
pci_update_resource(struct pci_dev *dev, struct resource *res, int resno)
{
struct pci_bus_region region;

pcibios_resource_to_bus(dev, &region, res);

new = region.start | (res->flags & PCI_REGION_FLAG_MASK);

if (resno < 6) {
reg = PCI_BASE_ADDRESS_0 + 4 * resno;
} else if (resno == PCI_ROM_RESOURCE) {
new |= res->flags & PCI_ROM_ADDRESS_ENABLE;
reg = dev->rom_base_reg;
}

pci_write_config_dword(dev, reg, new);
}

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-08-19 20:49:33

by Jon Smirl

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

--- Russell King <[email protected]> wrote:
> You should use pcibios_resource_to_bus() to convert a resource to a
> representation suitable for a BAR.

I've never used pcibios_resource_to_bus(), what's the right way to do this?
This is a good reason for making this into a common PCI driver function,
it will stop people like me from messing up PCI calls.




=====
Jon Smirl
[email protected]

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com

2003-08-19 21:18:01

by David Miller

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

On Tue, 19 Aug 2003 21:52:46 +0100
Russell King <[email protected]> wrote:

> new |= res->flags & PCI_ROM_ADDRESS_ENABLE;
> reg = dev->rom_base_reg;

A word of caution, please do not enable PCI ROMs lightly.

There are many devices which stop responding to MEM and IO
space once their ROM is enabled, Qlogic-ISP chips are one
such device and there are several others.

2003-08-19 21:38:43

by Russell King

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

On Tue, Aug 19, 2003 at 02:17:35PM -0700, David S. Miller wrote:
> On Tue, 19 Aug 2003 21:52:46 +0100
> Russell King <[email protected]> wrote:
> > new |= res->flags & PCI_ROM_ADDRESS_ENABLE;
> > reg = dev->rom_base_reg;
>
> A word of caution, please do not enable PCI ROMs lightly.
>
> There are many devices which stop responding to MEM and IO
> space once their ROM is enabled, Qlogic-ISP chips are one
> such device and there are several others.

Indeed - we leave the ROM enable bit in whatever state it was when
we scanned the device.

However, there are device drivers which want to access the ROM for
whatever reason, and we should provide a standard way to allow
drivers to enable / disable ROM access for architecture portability
reasons (so that VGA drivers can find tables in their ROMs for
instance.)

Since this is critical to some devices, maybe their drivers should
consider ensuring that the ROM resources are disabled upon driver
initialisation of the device?

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-08-19 21:58:56

by Jon Smirl

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

--- Russell King <[email protected]> wrote:
> On Tue, Aug 19, 2003 at 02:17:35PM -0700, David S. Miller wrote:
> > On Tue, 19 Aug 2003 21:52:46 +0100
> > Russell King <[email protected]> wrote:
> > > new |= res->flags & PCI_ROM_ADDRESS_ENABLE;
> > > reg = dev->rom_base_reg;
> >
> > A word of caution, please do not enable PCI ROMs lightly.
> >
> > There are many devices which stop responding to MEM and IO
> > space once their ROM is enabled, Qlogic-ISP chips are one
> > such device and there are several others.

I'm doing this in the device driver for the device, so I know it is ok to do.

> However, there are device drivers which want to access the ROM for
> whatever reason, and we should provide a standard way to allow
> drivers to enable / disable ROM access for architecture portability
> reasons (so that VGA drivers can find tables in their ROMs for
> instance.)

In my case I need access to tables in the ROM.

> Since this is critical to some devices, maybe their drivers should
> consider ensuring that the ROM resources are disabled upon driver
> initialisation of the device?

My driver can be load/unloaded so I need to turn the ROM on each
time to get to the tables. I also don't like calling release_resource()
directly from my driver since that looks like an internal PCI driver call.

As to pcibios_resource_to_bus()....

I called pci_assign_resource() which calls pci_update_resource()
which calls pcibios_resource_to_bus().

I see now that pci_assign_resource() will return with the ROM
enabled. I believe I have also hit cases where ROM was assigned
an address but not enabled. Do I still need to call
pcibios_resource_to_bus() if I am just enabling the ROM?

Do I need to call pcibios_resource_to_bus() when disabling the ROM?


=====
Jon Smirl
[email protected]

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com

2003-08-20 00:06:07

by Jamie Lokier

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

David S. Miller wrote:
> On Tue, 19 Aug 2003 21:52:46 +0100
> Russell King <[email protected]> wrote:
>
> > new |= res->flags & PCI_ROM_ADDRESS_ENABLE;
> > reg = dev->rom_base_reg;
>
> A word of caution, please do not enable PCI ROMs lightly.
>
> There are many devices which stop responding to MEM and IO
> space once their ROM is enabled, Qlogic-ISP chips are one
> such device and there are several others.

I take it the devices do respond to MEM and IO after the ROM is
disabled again?

-- Jamie

2003-08-20 00:12:13

by David Miller

[permalink] [raw]
Subject: Re: Standard driver call to enable/disable PCI ROM

On Wed, 20 Aug 2003 01:05:35 +0100
Jamie Lokier <[email protected]> wrote:

> David S. Miller wrote:
> > There are many devices which stop responding to MEM and IO
> > space once their ROM is enabled, Qlogic-ISP chips are one
> > such device and there are several others.
>
> I take it the devices do respond to MEM and IO after the ROM is
> disabled again?

That is correct.