[U-Boot] [PATCH v2] pci/fsl_pci_init: Enable inbound PCI config cycles

[Peter Tyser]

Hi Kumar,

> On Oct 28, 2008, at 7:24 PM, Peter Tyser wrote:
> 
>> Set CFG_READY bit in Configuration Ready register for PCIe
>> interfaces and clear ACL bit in PBFR register for PCI
>> interfaces to allow devices to respond to incoming PCI
>> configuration cycles.
>>
>> Signed-off-by: Peter Tyser <ptyser at xes-inc.com>
>> ---
>> Changes since v1:
>> - Anal retentive update to the commit message:)
>>
>> drivers/pci/fsl_pci_init.c |   17 +++++++++++++++++
>> 1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
>> index 7625ccc..6da9c22 100644
>> --- a/drivers/pci/fsl_pci_init.c
>> +++ b/drivers/pci/fsl_pci_init.c
>> @@ -37,6 +37,11 @@ DECLARE_GLOBAL_DATA_PTR;
>> #include <pci.h>
>> #include <asm/immap_fsl_pci.h>
>>
>> +/* Freescale-specific PCI config registers */
>> +#define FSL_PCI_PBFR        0x44
>> +#define FSL_PCIE_CAP_ID        0x4c
>> +#define FSL_PCIE_CFG_RDY    0x4b0
>> +
>> void pciauto_prescan_setup_bridge(struct pci_controller *hose,
>>                 pci_dev_t dev, int sub_bus);
>> void pciauto_postscan_setup_bridge(struct pci_controller *hose,
>> @@ -302,6 +307,18 @@ void fsl_pci_init(struct pci_controller *hose)
>>     if (temp16) {
>>         pci_hose_write_config_word(hose, dev, PCI_SEC_STATUS, 0xffff);
>>     }
>> +
>> +    /* Enable inbound PCI config cycles */
>> +    pci_hose_read_config_byte(hose, dev, FSL_PCIE_CAP_ID, &temp8);
>> +    if (temp8 != 0x0) {
>> +        /* PCIe - set CFG_READY bit of Configuration Ready Register */
>> +        pci_hose_write_config_byte(hose, dev, FSL_PCIE_CFG_RDY, 0x1);
>> +    } else {
>> +        /* PCI - clear ACL bit of PBFR */
>> +        pci_hose_read_config_word(hose, dev, FSL_PCI_PBFR, &temp16);
>> +        temp16 &= ~0x20;
>> +        pci_hose_write_config_byte(hose, dev, FSL_PCI_PBFR, temp16);
>> +    }
>> }
> 
> Shouldn't we only be doing this for an agent?  Also is the right place 
> to enable it?  Just wondering if board code should have more flexibility 
> here.

I was under the impression that if a PCIe interface was configured as 
root complex its CFG_READY bit would still need to be set in order for 
an endpoint device to perform config cycles to it (eg to see if the root 
complex device was sharing its memory, or if the root complex's shared 
memory was prefetchable, etc).  So I thought it made sense to 
unconditionally set the CFG_READY bit for PCIe interfaces.  Let me know 
if I'm missing something here (I thought some non-fsl chips could 
generate config cycles in endpoint mode)...

Looking at section 4.4.3.6 of the MPC8548 manual I thought that the ACL 
bit could be set on a host interface based on the value of the 
cfg_cpu_boot reset configuration bit.  Looking over the manual a bit 
more closely, it looks like if a PCI interface is configured as host it 
can't respond to inbound config cycles period (Table 16-65 of the 8548 
manual).  So for PCI hosts, ACL does not need to be cleared as you 
suggested.

As far as having more flexibility, I couldn't think of a reason that 
boards would want to have more fine grained control of this.  Is there 
an example you can think of where a board would want to do it differently?

Thanks for the comments,
Peter