linux-kernel - Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Dec 2011 15:20:03 -0500
From:	Josh Boyer <jwboyer@...il.com>
To:	Vinh Huu Tuong Nguyen <vhtnguyen@....com>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Matt Porter <mporter@...nel.crashing.org>,
	Kumar Gala <galak@...nel.crashing.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Anton Blanchard <anton@...ba.org>,
	Dave Kleikamp <shaggy@...ux.vnet.ibm.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Tony Breeds <tony@...eyournoodle.com>,
	Rob Herring <rob.herring@...xeda.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Lucas De Marchi <lucas.demarchi@...fusion.mobi>,
	Ayman El-Khashab <ayman@...hashab.com>,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and
 Bluestone board

On Tue, Dec 20, 2011 at 11:47 PM, Vinh Huu Tuong Nguyen
<vhtnguyen@....com> wrote:
>> +
>> +       mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> +               mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
>> +               (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
>> +
>> +       /* Poll for PHY reset */
>> +       timeout = 0;
>> +       while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> +                       (port->index * 0x55)) & 0x1)) &&
>> +                (timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> +               udelay(10);
>> +               timeout++;
>> +       }
>> +
>> +       if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> +               mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> +                       (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
>> +                       ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
>> +                       PESDRx_RCSSET_RSTPYN);
>> +
>> +               port->has_ibpre = 1;
>> +
>> +               return 0;
>> +       } else {
>> +               printk(KERN_INFO "PCIE: Can't reset PHY\n");
>> +               return -1;
>> +       }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc?  If so, you probably want to
> make that KERN_ERR.
> [Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
> properly. I will update codes to return the error in case PHY can't reset.

OK.

>> @@ -1751,9 +1856,9 @@ static void __init
> ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
>>                 * if it works
>>                 */
>>                out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
>> -               out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
>> +               out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving
> on HB */
>>                out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
>> -               out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
>> +               out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving
> on HB */
>>                out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
>>                out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
>
> Why are these values changed, and are those changes only needed on
> APM821xx?
> [Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
> SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we tried
> to initialize it for speedup. For APM821xx, although we didn't mention
> about this area, this configuration still works well. So we keep it.

This function isn't just called for 460EX or APM821xx SoCs.  It's
called on any 4xx SoC with PCIe configured.
Is this change going to work on them all?  If not, it needs to be
conditionalized.

>> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
> b/arch/powerpc/sysdev/ppc4xx_pci.h
>> index 32ce763..faf3017 100644
>> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
>> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
>> @@ -441,6 +441,7 @@
>>  /*
>>  * Config space register offsets
>>  */
>> +#define PECFG_ECDEVCTL         0x060
>>  #define PECFG_ECRTCTL          0x074
>>
>>  #define PECFG_BAR0LMPA         0x210
>> @@ -448,6 +449,7 @@
>>  #define PECFG_BAR1MPA          0x218
>>  #define PECFG_BAR2LMPA         0x220
>>  #define PECFG_BAR2HMPA         0x224
>> +#define PECFG_ECDEVCAPPA       0x25c
>>
>>  #define PECFG_PIMEN            0x33c
>>  #define PECFG_PIM0LAL          0x340
>> @@ -494,5 +496,7 @@ enum
>>        LNKW_X8                 = 0x8
>>  };
>>
>> +/* Timout for reset phy */
>> +#define PCIE_PHY_RESET_TIMEOUT 10
>
> Is this value applicable to all the 44x devices with PCI-e?
> [Vinh Nguyen] At this time, we only checked this on APM821xx chips.

Then it should probably be conditionalized somehow.  Or put as a
worst-case value that will work for them all.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/