From: Vinh Huu Tuong Nguyen <vhtnguyen@apm•com>
To: Josh Boyer <jwboyer@gmail•com>
Cc: Ayman El-Khashab <ayman@elkhashab•com>,
Dave Kleikamp <shaggy@linux•vnet.ibm.com>,
Lucas De Marchi <lucas.demarchi@profusion•mobi>,
Rob Herring <rob.herring@calxeda•com>,
Paul Gortmaker <paul.gortmaker@windriver•com>,
Paul Mackerras <paulus@samba•org>,
Anton Blanchard <anton@samba•org>, Jiri Kosina <jkosina@suse•cz>,
linuxppc-dev@lists•ozlabs.org, linux-kernel@vger•kernel.org
Subject: RE: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
Date: Mon, 26 Dec 2011 15:00:38 +0700 [thread overview]
Message-ID: <adc28ecd196e3cb9af86094e039f01b6@mail.gmail.com> (raw)
In-Reply-To: <CA+5PVA4cjiUvn4YavkFs6Jx87YpO8yHZZbm4pcPJQn1r1cggUA@mail.gmail.com>
-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail•com]
Sent: Thursday, December 22, 2011 3:20 AM
To: Vinh Huu Tuong Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Matt Porter; Kumar Gala; Paul
Gortmaker; Anton Blanchard; Dave Kleikamp; Grant Likely; Tony Breeds; Rob
Herring; Jiri Kosina; Lucas De Marchi; Ayman El-Khashab;
linuxppc-dev@lists•ozlabs.org; linux-kernel@vger•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@apm•com> wrote:
>> +
>> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mfdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPY=
N));
>> +
>> + =A0 =A0 =A0 /* Poll for PHY reset */
>> + =A0 =A0 =A0 timeout =3D 0;
>> + =A0 =A0 =A0 while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (port->index * 0x55)) & 0x=
1)) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout++;
>> + =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (mfdcri(SDR0, port->sdr_ba=
se + PESDRn_RCSSET) &
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~(PESDRx_RCSSET_RSTGU | PE=
SDRx_RCSSET_RSTDL)) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PESDRx_RCSSET_RSTPYN);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 port->has_ibpre =3D 1;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_INFO "PCIE: Can't reset PHY\n"=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
>> + =A0 =A0 =A0 }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc? =A0If 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,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * if it works
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM0LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000008=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM1LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x0000000c=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAH, 0xffff00=
00);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAL, 0x000000=
00);
>
> 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.
[Vinh Nguyen] I will update my codes as your comments. Please see next
submitted codes.
>> 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 @@
>> =A0/*
>> =A0* Config space register offsets
>> =A0*/
>> +#define PECFG_ECDEVCTL =A0 =A0 =A0 =A0 0x060
>> =A0#define PECFG_ECRTCTL =A0 =A0 =A0 =A0 =A00x074
>>
>> =A0#define PECFG_BAR0LMPA =A0 =A0 =A0 =A0 0x210
>> @@ -448,6 +449,7 @@
>> =A0#define PECFG_BAR1MPA =A0 =A0 =A0 =A0 =A00x218
>> =A0#define PECFG_BAR2LMPA =A0 =A0 =A0 =A0 0x220
>> =A0#define PECFG_BAR2HMPA =A0 =A0 =A0 =A0 0x224
>> +#define PECFG_ECDEVCAPPA =A0 =A0 =A0 0x25c
>>
>> =A0#define PECFG_PIMEN =A0 =A0 =A0 =A0 =A0 =A00x33c
>> =A0#define PECFG_PIM0LAL =A0 =A0 =A0 =A0 =A00x340
>> @@ -494,5 +496,7 @@ enum
>> =A0 =A0 =A0 =A0LNKW_X8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x8
>> =A0};
>>
>> +/* 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.
[Vinh Nguyen] At this time, this definition is called in
apm821xx_pciex_init_port_hw function that is used only for apm821xx chip.
So I think it's ok, but as your recommendation, I'll check with various
boards that I have to get the best value and update it in next submitted
codes.
Best regards,
Vinh Nguyen.
prev parent reply other threads:[~2011-12-26 8:00 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-20 12:44 [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board Vinh Nguyen Huu Tuong
2011-12-20 15:31 ` Josh Boyer
2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
2011-12-21 20:20 ` Josh Boyer
2011-12-26 8:00 ` Vinh Huu Tuong Nguyen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adc28ecd196e3cb9af86094e039f01b6@mail.gmail.com \
--to=vhtnguyen@apm$(echo .)com \
--cc=anton@samba$(echo .)org \
--cc=ayman@elkhashab$(echo .)com \
--cc=jkosina@suse$(echo .)cz \
--cc=jwboyer@gmail$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=lucas.demarchi@profusion$(echo .)mobi \
--cc=paul.gortmaker@windriver$(echo .)com \
--cc=paulus@samba$(echo .)org \
--cc=rob.herring@calxeda$(echo .)com \
--cc=shaggy@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox