On Sep 20, 2012, at 10:37 PM, Lian Minghaun-b31939 wrote:
> Hi Kumar,
>
> please see my comments inline.
>
>
> On 09/19/2012 10:22 PM, Kumar Gala wrote:
>> On Sep 19, 2012, at 2:23 AM, Minghuan Lian wrote:
>>
>>> The original code uses 'Programming Interface' field to judge if PCIE is
>>> EP or RC mode, however, some latest silicons do not support this
>>> functionality.
>>> According to PCIE specification, 'Header Type' offset 0x0e is used to
>>> indicate header type, so change code to use 'Header Type' field to
>>> judge PCIE mode. Because FSL PCI controller does not support 'Header Type',
>>> patch still uses 'Programming Interface' to identify PCI mode.
>>>
>>> Signed-off-by: Minghuan Lian <minghuan.l...@freescale.com>
>>> Signed-off-by: Roy Zang <tie-fei.z...@freescale.com>
>>> ---
>>> arch/powerpc/sysdev/fsl_pci.c | 38 +++++++++++++++++++++++---------------
>>> 1 file changed, 23 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
>>> index c37f461..43d30df 100644
>>> --- a/arch/powerpc/sysdev/fsl_pci.c
>>> +++ b/arch/powerpc/sysdev/fsl_pci.c
>>> @@ -38,15 +38,15 @@ static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
>>>
>>> static void __devinit quirk_fsl_pcie_header(struct pci_dev *dev)
>>> {
>>> - u8 progif;
>>> + u8 hdr_type;
>>>
>>> /* if we aren't a PCIe don't bother */
>>> if (!pci_find_capability(dev, PCI_CAP_ID_EXP))
>>> return;
>>>
>>> /* if we aren't in host mode don't bother */
>>> - pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
>>> - if (progif & 0x1)
>>> + pci_read_config_byte(dev, PCI_HEADER_TYPE, &hdr_type);
>>> + if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE)
>>> return;
>>>
>>> dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>>> @@ -425,7 +425,7 @@ int __init fsl_add_bridge(struct device_node *dev, int
>>> is_primary)
>>> struct pci_controller *hose;
>>> struct resource rsrc;
>>> const int *bus_range;
>>> - u8 progif;
>>> + u8 hdr_type, progif;
>>>
>>> if (!of_device_is_available(dev)) {
>>> pr_warning("%s: disabled\n", dev->full_name);
>>> @@ -457,25 +457,24 @@ int __init fsl_add_bridge(struct device_node *dev,
>>> int is_primary)
>>> setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
>>> PPC_INDIRECT_TYPE_BIG_ENDIAN);
>>>
>>> - early_read_config_byte(hose, 0, 0, PCI_CLASS_PROG, &progif);
>>> - if ((progif & 1) == 1) {
>>> - /* unmap cfg_data & cfg_addr separately if not on same page */
>>> - if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
>>> - ((unsigned long)hose->cfg_addr & PAGE_MASK))
>>> - iounmap(hose->cfg_data);
>>> - iounmap(hose->cfg_addr);
>>> - pcibios_free_controller(hose);
>>> - return -ENODEV;
>>> - }
>>> -
>>> setup_pci_cmd(hose);
>> I think we should be doing the check before we call setup_pci_cmd(). The
>> old code didn't touch the controller registers if we where and end-point.
>> We should maintain that.
> [Minghuan] Thanks for you pointing this.
> I want to move setup_pci_cmd like this:
>
> pr_debug(" ->Hose at 0x%p, cfg_addr=0x%p,cfg_data=0x%p\n",
> hose, hose->cfg_addr, hose->cfg_data);
>
> + setup_pci_cmd(hose);
>
> /* Interpret the "ranges" property */
> /* This also maps the I/O region and sets isa_io/mem_base */
> pci_process_bridge_OF_ranges(hose, dev, is_primary);
>
> This movement will cause fsl_pcie_check_link() calling before setup_pci_cmd().
> Is this ok?
I think so, as its how the code is today:
setup_pci_cmd()
..
if (pcie)
fsl_pcie_check_link()
> If not, I will call setup_pci_cmd() for PCI and PCIE respectively.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
