| Message ID | BLU437-SMTP9657DBF44BD40B81961776BFA00@phx.gbl |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Tom, On Tue, Jun 23, 2015 at 11:45 AM, Bin Meng <bmeng.cn@gmail.com> wrote: > Currently PCI expansion ROM address is assigned by a call to > pciauto_setup_rom() outside of the pci auto config process. > This does not work when expansion ROM is on a device behind > PCI bridge where bridge's memory limit register was already > programmed to a value that does not cover the newly assigned > expansion ROM address. To fix this, we should configure the > ROM address during the auto config process. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > Changes in v2: > - Handle header type 1 ROM address programming > > drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- > drivers/pci/pci_rom.c | 5 ----- > include/pci.h | 9 --------- > 3 files changed, 22 insertions(+), 40 deletions(-) > > diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c > index 7c10983..e034ed1 100644 > --- a/drivers/pci/pci_auto.c > +++ b/drivers/pci/pci_auto.c > @@ -87,6 +87,8 @@ void pciauto_setup_device(struct pci_controller *hose, > pci_size_t bar_size; > u16 cmdstat = 0; > int bar, bar_nr = 0; > + u8 header_type; > + int rom_addr; > #ifndef CONFIG_PCI_ENUM_ONLY > pci_addr_t bar_value; > struct pci_region *bar_res; > @@ -182,38 +184,32 @@ void pciauto_setup_device(struct pci_controller *hose, > bar_nr++; > } > > + /* Configure the expansion ROM address */ > + pci_hose_read_config_byte(hose, dev, PCI_HEADER_TYPE, &header_type); > + if (header_type != PCI_HEADER_TYPE_CARDBUS) { > + rom_addr = (header_type == PCI_HEADER_TYPE_NORMAL) ? > + PCI_ROM_ADDRESS : PCI_ROM_ADDRESS1; > + pci_hose_write_config_dword(hose, dev, rom_addr, 0xfffffffe); > + pci_hose_read_config_dword(hose, dev, rom_addr, &bar_response); > + if (bar_response) { > + bar_size = -(bar_response & ~1); > + DEBUGF("PCI Autoconfig: ROM, size=%#x, ", bar_size); > + if (pciauto_region_allocate(mem, bar_size, > + &bar_value) == 0) { > + pci_hose_write_config_dword(hose, dev, rom_addr, > + bar_value); > + } > + cmdstat |= PCI_COMMAND_MEMORY; > + DEBUGF("\n"); > + } > + } > + > pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); > pci_hose_write_config_byte(hose, dev, PCI_CACHE_LINE_SIZE, > CONFIG_SYS_PCI_CACHE_LINE_SIZE); > pci_hose_write_config_byte(hose, dev, PCI_LATENCY_TIMER, 0x80); > } > > -int pciauto_setup_rom(struct pci_controller *hose, pci_dev_t dev) > -{ > - pci_addr_t bar_value; > - pci_size_t bar_size; > - u32 bar_response; > - u16 cmdstat = 0; > - > - pci_hose_write_config_dword(hose, dev, PCI_ROM_ADDRESS, 0xfffffffe); > - pci_hose_read_config_dword(hose, dev, PCI_ROM_ADDRESS, &bar_response); > - if (!bar_response) > - return -ENOENT; > - > - bar_size = -(bar_response & ~1); > - DEBUGF("PCI Autoconfig: ROM, size=%#x, ", bar_size); > - if (pciauto_region_allocate(hose->pci_mem, bar_size, &bar_value) == 0) { > - pci_hose_write_config_dword(hose, dev, PCI_ROM_ADDRESS, > - bar_value); > - } > - DEBUGF("\n"); > - pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat); > - cmdstat |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; > - pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); > - > - return 0; > -} > - > void pciauto_prescan_setup_bridge(struct pci_controller *hose, > pci_dev_t dev, int sub_bus) > { > diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c > index 37450c8..f364799 100644 > --- a/drivers/pci/pci_rom.c > +++ b/drivers/pci/pci_rom.c > @@ -83,11 +83,6 @@ static int pci_rom_probe(pci_dev_t dev, uint class, > rom_address = CONFIG_X86_OPTION_ROM_ADDR; > #else > > - if (pciauto_setup_rom(pci_bus_to_hose(PCI_BUS(dev)), dev)) { > - debug("Cannot find option ROM\n"); > - return -ENOENT; > - } > - > pci_read_config_dword(dev, PCI_ROM_ADDRESS, &rom_address); > if (rom_address == 0x00000000 || rom_address == 0xffffffff) { > debug("%s: rom_address=%x\n", __func__, rom_address); > diff --git a/include/pci.h b/include/pci.h > index 07b1e9a..966be97 100644 > --- a/include/pci.h > +++ b/include/pci.h > @@ -712,15 +712,6 @@ void pci_write_bar32(struct pci_controller *hose, pci_dev_t dev, int barnum, > u32 pci_read_bar32(struct pci_controller *hose, pci_dev_t dev, int barnum); > > /** > - * pciauto_setup_rom() - Set up access to a device ROM > - * > - * @hose: PCI hose to use > - * @dev: PCI device to adjust > - * @return 0 if done, -ve on error > - */ > -int pciauto_setup_rom(struct pci_controller *hose, pci_dev_t dev); > - > -/** > * pci_hose_find_devices() - Find devices by vendor/device ID > * > * @hose: PCI hose to search > -- Ping? Regards, Bin
Hi Bin, On 22 June 2015 at 21:45, Bin Meng <bmeng.cn@gmail.com> wrote: > Currently PCI expansion ROM address is assigned by a call to > pciauto_setup_rom() outside of the pci auto config process. > This does not work when expansion ROM is on a device behind > PCI bridge where bridge's memory limit register was already > programmed to a value that does not cover the newly assigned > expansion ROM address. To fix this, we should configure the > ROM address during the auto config process. Well I should have known that approach would not work :-( > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > > Changes in v2: > - Handle header type 1 ROM address programming > > drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- > drivers/pci/pci_rom.c | 5 ----- > include/pci.h | 9 --------- > 3 files changed, 22 insertions(+), 40 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
Hi Tom, On Wed, Jul 1, 2015 at 7:17 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 22 June 2015 at 21:45, Bin Meng <bmeng.cn@gmail.com> wrote: >> Currently PCI expansion ROM address is assigned by a call to >> pciauto_setup_rom() outside of the pci auto config process. >> This does not work when expansion ROM is on a device behind >> PCI bridge where bridge's memory limit register was already >> programmed to a value that does not cover the newly assigned >> expansion ROM address. To fix this, we should configure the >> ROM address during the auto config process. > > Well I should have known that approach would not work :-( > >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> >> --- >> >> Changes in v2: >> - Handle header type 1 ROM address programming >> >> drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- >> drivers/pci/pci_rom.c | 5 ----- >> include/pci.h | 9 --------- >> 3 files changed, 22 insertions(+), 40 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> Will you apply these two patches for v2015.07 release, or you want to leave them to next? Regards, Bin
Hi Bin, On 6 July 2015 at 02:12, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Tom, > > On Wed, Jul 1, 2015 at 7:17 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 22 June 2015 at 21:45, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Currently PCI expansion ROM address is assigned by a call to >>> pciauto_setup_rom() outside of the pci auto config process. >>> This does not work when expansion ROM is on a device behind >>> PCI bridge where bridge's memory limit register was already >>> programmed to a value that does not cover the newly assigned >>> expansion ROM address. To fix this, we should configure the >>> ROM address during the auto config process. >> >> Well I should have known that approach would not work :-( >> >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> >>> --- >>> >>> Changes in v2: >>> - Handle header type 1 ROM address programming >>> >>> drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- >>> drivers/pci/pci_rom.c | 5 ----- >>> include/pci.h | 9 --------- >>> 3 files changed, 22 insertions(+), 40 deletions(-) >> >> Reviewed-by: Simon Glass <sjg@chromium.org> > > Will you apply these two patches for v2015.07 release, or you want to > leave them to next? I've already brought in patches to x86/master that I was not planning to send to mainline for this release (no concerns about the patches, just merge window timing). But I could put these into another branch and sent a pull request if you like Bin? My understanding was that this fix did not affect any current boards / functionality, but I may have that wrong. Regards, Simon
Hi Simon, On Mon, Jul 6, 2015 at 8:51 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 6 July 2015 at 02:12, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Tom, >> >> On Wed, Jul 1, 2015 at 7:17 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 22 June 2015 at 21:45, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Currently PCI expansion ROM address is assigned by a call to >>>> pciauto_setup_rom() outside of the pci auto config process. >>>> This does not work when expansion ROM is on a device behind >>>> PCI bridge where bridge's memory limit register was already >>>> programmed to a value that does not cover the newly assigned >>>> expansion ROM address. To fix this, we should configure the >>>> ROM address during the auto config process. >>> >>> Well I should have known that approach would not work :-( >>> >>>> >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> >>>> --- >>>> >>>> Changes in v2: >>>> - Handle header type 1 ROM address programming >>>> >>>> drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- >>>> drivers/pci/pci_rom.c | 5 ----- >>>> include/pci.h | 9 --------- >>>> 3 files changed, 22 insertions(+), 40 deletions(-) >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> Will you apply these two patches for v2015.07 release, or you want to >> leave them to next? > > I've already brought in patches to x86/master that I was not planning > to send to mainline for this release (no concerns about the patches, > just merge window timing). > > But I could put these into another branch and sent a pull request if > you like Bin? My understanding was that this fix did not affect any > current boards / functionality, but I may have that wrong. > Your understanding is correct. This fix did not affect any current board. It's just because I saw this patch was assigned to Tom previously so I assume it would be taken by Tom directly for this release. Just want to double check. I am OK for next release. Regards, Bin
diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c index 7c10983..e034ed1 100644 --- a/drivers/pci/pci_auto.c +++ b/drivers/pci/pci_auto.c @@ -87,6 +87,8 @@ void pciauto_setup_device(struct pci_controller *hose, pci_size_t bar_size; u16 cmdstat = 0; int bar, bar_nr = 0; + u8 header_type; + int rom_addr; #ifndef CONFIG_PCI_ENUM_ONLY pci_addr_t bar_value; struct pci_region *bar_res; @@ -182,38 +184,32 @@ void pciauto_setup_device(struct pci_controller *hose, bar_nr++; } + /* Configure the expansion ROM address */ + pci_hose_read_config_byte(hose, dev, PCI_HEADER_TYPE, &header_type); + if (header_type != PCI_HEADER_TYPE_CARDBUS) { + rom_addr = (header_type == PCI_HEADER_TYPE_NORMAL) ? + PCI_ROM_ADDRESS : PCI_ROM_ADDRESS1; + pci_hose_write_config_dword(hose, dev, rom_addr, 0xfffffffe); + pci_hose_read_config_dword(hose, dev, rom_addr, &bar_response); + if (bar_response) { + bar_size = -(bar_response & ~1); + DEBUGF("PCI Autoconfig: ROM, size=%#x, ", bar_size); + if (pciauto_region_allocate(mem, bar_size, + &bar_value) == 0) { + pci_hose_write_config_dword(hose, dev, rom_addr, + bar_value); + } + cmdstat |= PCI_COMMAND_MEMORY; + DEBUGF("\n"); + } + } + pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); pci_hose_write_config_byte(hose, dev, PCI_CACHE_LINE_SIZE, CONFIG_SYS_PCI_CACHE_LINE_SIZE); pci_hose_write_config_byte(hose, dev, PCI_LATENCY_TIMER, 0x80); } -int pciauto_setup_rom(struct pci_controller *hose, pci_dev_t dev) -{ - pci_addr_t bar_value; - pci_size_t bar_size; - u32 bar_response; - u16 cmdstat = 0; - - pci_hose_write_config_dword(hose, dev, PCI_ROM_ADDRESS, 0xfffffffe); - pci_hose_read_config_dword(hose, dev, PCI_ROM_ADDRESS, &bar_response); - if (!bar_response) - return -ENOENT; - - bar_size = -(bar_response & ~1); - DEBUGF("PCI Autoconfig: ROM, size=%#x, ", bar_size); - if (pciauto_region_allocate(hose->pci_mem, bar_size, &bar_value) == 0) { - pci_hose_write_config_dword(hose, dev, PCI_ROM_ADDRESS, - bar_value); - } - DEBUGF("\n"); - pci_hose_read_config_word(hose, dev, PCI_COMMAND, &cmdstat); - cmdstat |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; - pci_hose_write_config_word(hose, dev, PCI_COMMAND, cmdstat); - - return 0; -} - void pciauto_prescan_setup_bridge(struct pci_controller *hose, pci_dev_t dev, int sub_bus) { diff --git a/drivers/pci/pci_rom.c b/drivers/pci/pci_rom.c index 37450c8..f364799 100644 --- a/drivers/pci/pci_rom.c +++ b/drivers/pci/pci_rom.c @@ -83,11 +83,6 @@ static int pci_rom_probe(pci_dev_t dev, uint class, rom_address = CONFIG_X86_OPTION_ROM_ADDR; #else - if (pciauto_setup_rom(pci_bus_to_hose(PCI_BUS(dev)), dev)) { - debug("Cannot find option ROM\n"); - return -ENOENT; - } - pci_read_config_dword(dev, PCI_ROM_ADDRESS, &rom_address); if (rom_address == 0x00000000 || rom_address == 0xffffffff) { debug("%s: rom_address=%x\n", __func__, rom_address); diff --git a/include/pci.h b/include/pci.h index 07b1e9a..966be97 100644 --- a/include/pci.h +++ b/include/pci.h @@ -712,15 +712,6 @@ void pci_write_bar32(struct pci_controller *hose, pci_dev_t dev, int barnum, u32 pci_read_bar32(struct pci_controller *hose, pci_dev_t dev, int barnum); /** - * pciauto_setup_rom() - Set up access to a device ROM - * - * @hose: PCI hose to use - * @dev: PCI device to adjust - * @return 0 if done, -ve on error - */ -int pciauto_setup_rom(struct pci_controller *hose, pci_dev_t dev); - -/** * pci_hose_find_devices() - Find devices by vendor/device ID * * @hose: PCI hose to search
Currently PCI expansion ROM address is assigned by a call to pciauto_setup_rom() outside of the pci auto config process. This does not work when expansion ROM is on a device behind PCI bridge where bridge's memory limit register was already programmed to a value that does not cover the newly assigned expansion ROM address. To fix this, we should configure the ROM address during the auto config process. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- Changes in v2: - Handle header type 1 ROM address programming drivers/pci/pci_auto.c | 48 ++++++++++++++++++++++-------------------------- drivers/pci/pci_rom.c | 5 ----- include/pci.h | 9 --------- 3 files changed, 22 insertions(+), 40 deletions(-)