* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio
@ 2017-09-12 17:56 Joakim Tjernlund
2017-09-14 21:15 ` York Sun
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Joakim Tjernlund @ 2017-09-12 17:56 UTC (permalink / raw)
To: u-boot
Most FSL PCIe controllers expects 333 MHz PCI reference clock.
This clock is derived from the CCB but in many cases the ref.
clock is not 333 MHz and a divisor needs to be configured.
This adds PEX_CCB_DIV #define which can be defined for each
type of CPU/platform.
Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
drivers/pci/fsl_pci_init.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
index 52792dcd59..4d00b3f26c 100644
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info)
pci_setup_indirect(hose, cfg_addr, cfg_data);
+#ifdef PEX_CCB_DIV
+ /* Configure the PCIE controller core clock ratio */
+ pci_hose_write_config_dword(hose, dev, 0x440,
+ ((gd->bus_clk / 1000000) *
+ (16 / PEX_CCB_DIV)) / 333);
+#endif
block_rev = in_be32(&pci->block_rev1);
if (PEX_IP_BLK_REV_2_2 <= block_rev) {
pi = &pci->pit[2]; /* 0xDC0 */
--
2.13.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-09-12 17:56 [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio Joakim Tjernlund @ 2017-09-14 21:15 ` York Sun 2017-09-15 6:14 ` Mingkai Hu 2017-11-21 17:18 ` Joakim Tjernlund 2018-08-13 16:29 ` York Sun 2 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2017-09-14 21:15 UTC (permalink / raw) To: u-boot On 09/12/2017 10:56 AM, Joakim Tjernlund wrote: > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > This clock is derived from the CCB but in many cases the ref. > clock is not 333 MHz and a divisor needs to be configured. > > This adds PEX_CCB_DIV #define which can be defined for each > type of CPU/platform. > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- > drivers/pci/fsl_pci_init.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > index 52792dcd59..4d00b3f26c 100644 > --- a/drivers/pci/fsl_pci_init.c > +++ b/drivers/pci/fsl_pci_init.c > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > +#ifdef PEX_CCB_DIV > + /* Configure the PCIE controller core clock ratio */ > + pci_hose_write_config_dword(hose, dev, 0x440, > + ((gd->bus_clk / 1000000) * > + (16 / PEX_CCB_DIV)) / 333); > +#endif > block_rev = in_be32(&pci->block_rev1); > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > pi = &pci->pit[2]; /* 0xDC0 */ > Mingkai, Do you ack this change? This presumes the PCIe clock derives from CCB bus clock. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-09-14 21:15 ` York Sun @ 2017-09-15 6:14 ` Mingkai Hu 0 siblings, 0 replies; 21+ messages in thread From: Mingkai Hu @ 2017-09-15 6:14 UTC (permalink / raw) To: u-boot > -----Original Message----- > From: York Sun > Sent: Friday, September 15, 2017 5:15 AM > To: Joakim Tjernlund <joakim.tjernlund@infinera.com>; Mingkai Hu > <mingkai.hu@nxp.com>; u-boot @ lists . denx . de <u-boot@lists.denx.de>; > Roy Zang <roy.zang@nxp.com> > Subject: Re: [PATCH] FSL PCI: Configure PCIe reference ratio > > On 09/12/2017 10:56 AM, Joakim Tjernlund wrote: > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > This clock is derived from the CCB but in many cases the ref. > > clock is not 333 MHz and a divisor needs to be configured. > > > > This adds PEX_CCB_DIV #define which can be defined for each type of > > CPU/platform. > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > --- > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > index 52792dcd59..4d00b3f26c 100644 > > --- a/drivers/pci/fsl_pci_init.c > > +++ b/drivers/pci/fsl_pci_init.c > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, > > struct fsl_pci_info *pci_info) > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > +#ifdef PEX_CCB_DIV > > + /* Configure the PCIE controller core clock ratio */ > > + pci_hose_write_config_dword(hose, dev, 0x440, > > + ((gd->bus_clk / 1000000) * > > + (16 / PEX_CCB_DIV)) / 333); > > +#endif > > block_rev = in_be32(&pci->block_rev1); > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > Mingkai, > > Do you ack this change? This presumes the PCIe clock derives from CCB bus > clock. > gd->bus_clk indicates the platform clock while PCIe clock could be CCB or CCB/2. For example, it's CCB/2 for T1040/T1020, CCB for P2020. I suggest to add the PCIe clock in gd to handle this difference, just like what we've done for other IP clocks. Thanks, Mingkai ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-09-12 17:56 [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio Joakim Tjernlund 2017-09-14 21:15 ` York Sun @ 2017-11-21 17:18 ` Joakim Tjernlund 2017-11-21 17:23 ` York Sun 2018-08-13 16:29 ` York Sun 2 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 17:18 UTC (permalink / raw) To: u-boot On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > This clock is derived from the CCB but in many cases the ref. > clock is not 333 MHz and a divisor needs to be configured. > > This adds PEX_CCB_DIV #define which can be defined for each > type of CPU/platform. > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- > drivers/pci/fsl_pci_init.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > index 52792dcd59..4d00b3f26c 100644 > --- a/drivers/pci/fsl_pci_init.c > +++ b/drivers/pci/fsl_pci_init.c > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > +#ifdef PEX_CCB_DIV > + /* Configure the PCIE controller core clock ratio */ > + pci_hose_write_config_dword(hose, dev, 0x440, > + ((gd->bus_clk / 1000000) * > + (16 / PEX_CCB_DIV)) / 333); > +#endif > block_rev = in_be32(&pci->block_rev1); > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > pi = &pci->pit[2]; /* 0xDC0 */ Ping? Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:18 ` Joakim Tjernlund @ 2017-11-21 17:23 ` York Sun 2017-11-21 17:29 ` Joakim Tjernlund 0 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2017-11-21 17:23 UTC (permalink / raw) To: u-boot On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >> This clock is derived from the CCB but in many cases the ref. >> clock is not 333 MHz and a divisor needs to be configured. >> >> This adds PEX_CCB_DIV #define which can be defined for each >> type of CPU/platform. >> >> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >> --- >> drivers/pci/fsl_pci_init.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >> index 52792dcd59..4d00b3f26c 100644 >> --- a/drivers/pci/fsl_pci_init.c >> +++ b/drivers/pci/fsl_pci_init.c >> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >> >> pci_setup_indirect(hose, cfg_addr, cfg_data); >> >> +#ifdef PEX_CCB_DIV >> + /* Configure the PCIE controller core clock ratio */ >> + pci_hose_write_config_dword(hose, dev, 0x440, >> + ((gd->bus_clk / 1000000) * >> + (16 / PEX_CCB_DIV)) / 333); >> +#endif >> block_rev = in_be32(&pci->block_rev1); >> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >> pi = &pci->pit[2]; /* 0xDC0 */ > > Ping? > > Jocke > I believe Mingkai's last comment was "to add the PCIe clock in gd". York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:23 ` York Sun @ 2017-11-21 17:29 ` Joakim Tjernlund 2017-11-21 17:32 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 17:29 UTC (permalink / raw) To: u-boot On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > This clock is derived from the CCB but in many cases the ref. > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > type of CPU/platform. > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > --- > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > index 52792dcd59..4d00b3f26c 100644 > > > --- a/drivers/pci/fsl_pci_init.c > > > +++ b/drivers/pci/fsl_pci_init.c > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > +#ifdef PEX_CCB_DIV > > > + /* Configure the PCIE controller core clock ratio */ > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > + ((gd->bus_clk / 1000000) * > > > + (16 / PEX_CCB_DIV)) / 333); > > > +#endif > > > block_rev = in_be32(&pci->block_rev1); > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > Ping? > > > > Jocke > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". Right, I seem to have forgotten to comment on that ... Why add that complexity/bloat? This is a constant defined by the CPU/SOC so it makes perfect sense to just #define it. Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:29 ` Joakim Tjernlund @ 2017-11-21 17:32 ` York Sun 2017-11-21 17:41 ` Joakim Tjernlund 0 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2017-11-21 17:32 UTC (permalink / raw) To: u-boot On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >> >> >> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>> This clock is derived from the CCB but in many cases the ref. >>>> clock is not 333 MHz and a divisor needs to be configured. >>>> >>>> This adds PEX_CCB_DIV #define which can be defined for each >>>> type of CPU/platform. >>>> >>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>> --- >>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>> index 52792dcd59..4d00b3f26c 100644 >>>> --- a/drivers/pci/fsl_pci_init.c >>>> +++ b/drivers/pci/fsl_pci_init.c >>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>> >>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>> >>>> +#ifdef PEX_CCB_DIV >>>> + /* Configure the PCIE controller core clock ratio */ >>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>> + ((gd->bus_clk / 1000000) * >>>> + (16 / PEX_CCB_DIV)) / 333); >>>> +#endif >>>> block_rev = in_be32(&pci->block_rev1); >>>> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >>>> pi = &pci->pit[2]; /* 0xDC0 */ >>> >>> Ping? >>> >>> Jocke >>> >> >> I believe Mingkai's last comment was "to add the PCIe clock in gd". > > Right, I seem to have forgotten to comment on that ... > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > it makes perfect sense to just #define it. > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. Would it be better if you use a Kconfig option and select the divisor by SoC? York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:32 ` York Sun @ 2017-11-21 17:41 ` Joakim Tjernlund 2017-11-21 17:45 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 17:41 UTC (permalink / raw) To: u-boot On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > type of CPU/platform. > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > --- > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > + ((gd->bus_clk / 1000000) * > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > +#endif > > > > > block_rev = in_be32(&pci->block_rev1); > > > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > > > > > Ping? > > > > > > > > Jocke > > > > > > > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". > > > > Right, I seem to have forgotten to comment on that ... > > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > > it makes perfect sense to just #define it. > > > > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. > Would it be better if you use a Kconfig option and select the divisor by > SoC? No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define in relevant PPC CPU, like in config_mpc85xx.h Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:41 ` Joakim Tjernlund @ 2017-11-21 17:45 ` York Sun 2017-11-21 17:51 ` Joakim Tjernlund 0 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2017-11-21 17:45 UTC (permalink / raw) To: u-boot On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >> >> >> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>> >>>> >>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>> >>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>> type of CPU/platform. >>>>>> >>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>> --- >>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>> 1 file changed, 6 insertions(+) >>>>>> >>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>> >>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>> >>>>>> +#ifdef PEX_CCB_DIV >>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>> + ((gd->bus_clk / 1000000) * >>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>> +#endif >>>>>> block_rev = in_be32(&pci->block_rev1); >>>>>> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >>>>>> pi = &pci->pit[2]; /* 0xDC0 */ >>>>> >>>>> Ping? >>>>> >>>>> Jocke >>>>> >>>> >>>> I believe Mingkai's last comment was "to add the PCIe clock in gd". >>> >>> Right, I seem to have forgotten to comment on that ... >>> Why add that complexity/bloat? This is a constant defined by the CPU/SOC so >>> it makes perfect sense to just #define it. >>> >> >> I am leaning to agree with you. The clock is either CCB clock, or CCB/2. >> Would it be better if you use a Kconfig option and select the divisor by >> SoC? > > No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define > in relevant PPC CPU, like in config_mpc85xx.h > So what should be in Kconfig, and what shouldn't? This is per SoC macro. Sounds like CONFIG_SYS_ in old days. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:45 ` York Sun @ 2017-11-21 17:51 ` Joakim Tjernlund 2017-11-21 18:04 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 17:51 UTC (permalink / raw) To: u-boot On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: > > On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > > > type of CPU/platform. > > > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > > > --- > > > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > > > + ((gd->bus_clk / 1000000) * > > > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > > > +#endif > > > > > > > block_rev = in_be32(&pci->block_rev1); > > > > > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > > > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > > > > > > > > > Ping? > > > > > > > > > > > > Jocke > > > > > > > > > > > > > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". > > > > > > > > Right, I seem to have forgotten to comment on that ... > > > > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > > > > it makes perfect sense to just #define it. > > > > > > > > > > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. > > > Would it be better if you use a Kconfig option and select the divisor by > > > SoC? > > > > No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define > > in relevant PPC CPU, like in config_mpc85xx.h > > > > So what should be in Kconfig, and what shouldn't? This is per SoC macro. > Sounds like CONFIG_SYS_ in old days. I must confess, I am not up to date with Kconfig stuff. I based my suggestion on what already exists in config_mpc85xx.h: #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) #define CONFIG_E5500 #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ #ifdef CONFIG_SYS_FSL_DDR4 #define CONFIG_SYS_FSL_DDRC_GEN4 #endif #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) #define CONFIG_MAX_CPUS 4 #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) #define CONFIG_MAX_CPUS 2 #endif #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } #define CONFIG_SYS_FSL_NUM_LAWS 16 #define CONFIG_SYS_FSL_SRDS_1 #define CONFIG_SYS_FSL_SEC_COMPAT 5 #define CONFIG_SYS_NUM_FMAN 1 #define CONFIG_SYS_NUM_FM1_DTSEC 5 .... etc. Seems like a good place to put another CPU constant. Anyhow, that patch stands of its own I think. Where to put all constants is another patch for NXP. Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 17:51 ` Joakim Tjernlund @ 2017-11-21 18:04 ` York Sun 2017-11-21 18:20 ` Joakim Tjernlund 0 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2017-11-21 18:04 UTC (permalink / raw) To: u-boot On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: >> >> On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>> >>>> >>>> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>> >>>>>> >>>>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>>>> >>>>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>>>> type of CPU/platform. >>>>>>>> >>>>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>>>> --- >>>>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>>>> >>>>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>>>> >>>>>>>> +#ifdef PEX_CCB_DIV >>>>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>>>> + ((gd->bus_clk / 1000000) * >>>>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>>>> +#endif I took another look at this patch. Would it be appropriate to alway write to this register with correct clock? >>>>>>>> block_rev = in_be32(&pci->block_rev1); >>>>>>>> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >>>>>>>> pi = &pci->pit[2]; /* 0xDC0 */ >>>>>>> >>>>>>> Ping? >>>>>>> >>>>>>> Jocke >>>>>>> >>>>>> >>>>>> I believe Mingkai's last comment was "to add the PCIe clock in gd". >>>>> >>>>> Right, I seem to have forgotten to comment on that ... >>>>> Why add that complexity/bloat? This is a constant defined by the CPU/SOC so >>>>> it makes perfect sense to just #define it. >>>>> >>>> >>>> I am leaning to agree with you. The clock is either CCB clock, or CCB/2. >>>> Would it be better if you use a Kconfig option and select the divisor by >>>> SoC? >>> >>> No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define >>> in relevant PPC CPU, like in config_mpc85xx.h >>> >> >> So what should be in Kconfig, and what shouldn't? This is per SoC macro. >> Sounds like CONFIG_SYS_ in old days. > > I must confess, I am not up to date with Kconfig stuff. I based my suggestion on > what already exists in config_mpc85xx.h: > #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ > defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > #define CONFIG_E5500 > #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ > #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ > #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 > #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ > #ifdef CONFIG_SYS_FSL_DDR4 > #define CONFIG_SYS_FSL_DDRC_GEN4 > #endif > #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) > #define CONFIG_MAX_CPUS 4 > #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > #define CONFIG_MAX_CPUS 2 > #endif > #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 > #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } > #define CONFIG_SYS_FSL_NUM_LAWS 16 > #define CONFIG_SYS_FSL_SRDS_1 > #define CONFIG_SYS_FSL_SEC_COMPAT 5 > #define CONFIG_SYS_NUM_FMAN 1 > #define CONFIG_SYS_NUM_FM1_DTSEC 5 > .... > etc. > > Seems like a good place to put another CPU constant. > Yeah. We have been moving things out of header files and into Kconfig. I would avoid adding new macro to header files. > Anyhow, that patch stands of its own I think. Where to put all constants > is another patch for NXP. Yes, we can add another patch to define this option for SoCs. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 18:04 ` York Sun @ 2017-11-21 18:20 ` Joakim Tjernlund 2017-11-21 18:35 ` York Sun 2018-02-27 19:30 ` York Sun 0 siblings, 2 replies; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 18:20 UTC (permalink / raw) To: u-boot On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: > > > On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: > > On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: > > > > > > On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > > > > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > > > > > type of CPU/platform. > > > > > > > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > > > > > --- > > > > > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > > > > > + ((gd->bus_clk / 1000000) * > > > > > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > > > > > +#endif > > > > I took another look at this patch. Would it be appropriate to alway > write to this register with correct clock? I sure hope so, the docs I have only mentions this reg having a default value which needs to be changed in most cases I guess. > > > > > > > > > > > > block_rev = in_be32(&pci->block_rev1); > > > > > > > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > > > > > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > > > > > > > > > > > > > Ping? > > > > > > > > > > > > > > > > Jocke > > > > > > > > > > > > > > > > > > > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". > > > > > > > > > > > > Right, I seem to have forgotten to comment on that ... > > > > > > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > > > > > > it makes perfect sense to just #define it. > > > > > > > > > > > > > > > > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. > > > > > Would it be better if you use a Kconfig option and select the divisor by > > > > > SoC? > > > > > > > > No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define > > > > in relevant PPC CPU, like in config_mpc85xx.h > > > > > > > > > > So what should be in Kconfig, and what shouldn't? This is per SoC macro. > > > Sounds like CONFIG_SYS_ in old days. > > > > I must confess, I am not up to date with Kconfig stuff. I based my suggestion on > > what already exists in config_mpc85xx.h: > > #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ > > defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > #define CONFIG_E5500 > > #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ > > #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ > > #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 > > #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ > > #ifdef CONFIG_SYS_FSL_DDR4 > > #define CONFIG_SYS_FSL_DDRC_GEN4 > > #endif > > #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) > > #define CONFIG_MAX_CPUS 4 > > #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > #define CONFIG_MAX_CPUS 2 > > #endif > > #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 > > #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } > > #define CONFIG_SYS_FSL_NUM_LAWS 16 > > #define CONFIG_SYS_FSL_SRDS_1 > > #define CONFIG_SYS_FSL_SEC_COMPAT 5 > > #define CONFIG_SYS_NUM_FMAN 1 > > #define CONFIG_SYS_NUM_FM1_DTSEC 5 > > .... > > etc. > > > > Seems like a good place to put another CPU constant. > > > > Yeah. We have been moving things out of header files and into Kconfig. I > would avoid adding new macro to header files. OK, I am happy if don't have to manually select the constant. > > > Anyhow, that patch stands of its own I think. Where to put all constants > > is another patch for NXP. > > Yes, we can add another patch to define this option for SoCs. :) Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 18:20 ` Joakim Tjernlund @ 2017-11-21 18:35 ` York Sun 2017-11-21 18:39 ` Joakim Tjernlund 2018-02-27 19:30 ` York Sun 1 sibling, 1 reply; 21+ messages in thread From: York Sun @ 2017-11-21 18:35 UTC (permalink / raw) To: u-boot On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: >> >> >> On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: >>>> >>>> On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>> >>>>>> >>>>>> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>>>>>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>> >>>>>>>> >>>>>>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>>>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>>>>>> >>>>>>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>>>>>> type of CPU/platform. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>>>>>> --- >>>>>>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>>>>>> >>>>>>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>>>>>> >>>>>>>>>> +#ifdef PEX_CCB_DIV >>>>>>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>>>>>> + ((gd->bus_clk / 1000000) * >>>>>>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>>>>>> +#endif >> >> >> >> I took another look at this patch. Would it be appropriate to alway >> write to this register with correct clock? > > I sure hope so, the docs I have only mentions this reg having a default value which > needs to be changed in most cases I guess. The only case we don't need to set this register is when the actual clock is 333MHz. I wonder why we didn't have a problem so far. Maybe we just didn't realize it. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 18:35 ` York Sun @ 2017-11-21 18:39 ` Joakim Tjernlund 2017-11-21 18:41 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2017-11-21 18:39 UTC (permalink / raw) To: u-boot On Tue, 2017-11-21 at 18:35 +0000, York Sun wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: > > On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: > > > > > > > > > On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: > > > > > > > > > > On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > > > > > > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > > > > > > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > > > > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > > > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > > > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > > > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > > > > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > > > > > > > type of CPU/platform. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > > > > > > > --- > > > > > > > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > > > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > > > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > > > > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > > > > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > > > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > > > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > > > > > > > + ((gd->bus_clk / 1000000) * > > > > > > > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > > > > > > > +#endif > > > > > > > > > > > > I took another look at this patch. Would it be appropriate to alway > > > write to this register with correct clock? > > > > I sure hope so, the docs I have only mentions this reg having a default value which > > needs to be changed in most cases I guess. > > > The only case we don't need to set this register is when the actual > clock is 333MHz. I wonder why we didn't have a problem so far. Maybe we > just didn't realize it. Have you tried higher PCI speeds? I remember a board we had didn't work with higher speeds so we settled for less than max. Don't hav the board handy now. Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 18:39 ` Joakim Tjernlund @ 2017-11-21 18:41 ` York Sun 0 siblings, 0 replies; 21+ messages in thread From: York Sun @ 2017-11-21 18:41 UTC (permalink / raw) To: u-boot On 11/21/2017 10:40 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 18:35 +0000, York Sun wrote: >> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >> >> >> On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: >>>> >>>> >>>> On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: >>>>>> >>>>>> On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: >>>>>>> On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>> >>>>>>>> >>>>>>>> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>>>>>>>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>>>>>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>>>>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>>>>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>>>>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>>>>>>>> >>>>>>>>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>>>>>>>> type of CPU/platform. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>>>>>>>> --- >>>>>>>>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>>>>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>>>>>>>> >>>>>>>>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>>>>>>>> >>>>>>>>>>>> +#ifdef PEX_CCB_DIV >>>>>>>>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>>>>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>>>>>>>> + ((gd->bus_clk / 1000000) * >>>>>>>>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>>>>>>>> +#endif >>>> >>>> >>>> >>>> I took another look at this patch. Would it be appropriate to alway >>>> write to this register with correct clock? >>> >>> I sure hope so, the docs I have only mentions this reg having a default value which >>> needs to be changed in most cases I guess. >> >> >> The only case we don't need to set this register is when the actual >> clock is 333MHz. I wonder why we didn't have a problem so far. Maybe we >> just didn't realize it. > > Have you tried higher PCI speeds? I remember a board we had didn't work with higher > speeds so we settled for less than max. Don't hav the board handy now. > No, I haven't. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-11-21 18:20 ` Joakim Tjernlund 2017-11-21 18:35 ` York Sun @ 2018-02-27 19:30 ` York Sun 2018-02-27 19:52 ` Joakim Tjernlund 1 sibling, 1 reply; 21+ messages in thread From: York Sun @ 2018-02-27 19:30 UTC (permalink / raw) To: u-boot On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: > On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: >> >> >> On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: >>>> >>>> On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>> >>>>>> >>>>>> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>>>>>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>> >>>>>>>> >>>>>>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>>>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>>>>>> >>>>>>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>>>>>> type of CPU/platform. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>>>>>> --- >>>>>>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>>>>>> >>>>>>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>>>>>> >>>>>>>>>> +#ifdef PEX_CCB_DIV >>>>>>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>>>>>> + ((gd->bus_clk / 1000000) * >>>>>>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>>>>>> +#endif >> >> >> >> I took another look at this patch. Would it be appropriate to alway >> write to this register with correct clock? > > I sure hope so, the docs I have only mentions this reg having a default value which > needs to be changed in most cases I guess. > >> >> >> >>>>>>>>>> block_rev = in_be32(&pci->block_rev1); >>>>>>>>>> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >>>>>>>>>> pi = &pci->pit[2]; /* 0xDC0 */ >>>>>>>>> >>>>>>>>> Ping? >>>>>>>>> >>>>>>>>> Jocke >>>>>>>>> >>>>>>>> >>>>>>>> I believe Mingkai's last comment was "to add the PCIe clock in gd". >>>>>>> >>>>>>> Right, I seem to have forgotten to comment on that ... >>>>>>> Why add that complexity/bloat? This is a constant defined by the CPU/SOC so >>>>>>> it makes perfect sense to just #define it. >>>>>>> >>>>>> >>>>>> I am leaning to agree with you. The clock is either CCB clock, or CCB/2. >>>>>> Would it be better if you use a Kconfig option and select the divisor by >>>>>> SoC? >>>>> >>>>> No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define >>>>> in relevant PPC CPU, like in config_mpc85xx.h >>>>> >>>> >>>> So what should be in Kconfig, and what shouldn't? This is per SoC macro. >>>> Sounds like CONFIG_SYS_ in old days. >>> >>> I must confess, I am not up to date with Kconfig stuff. I based my suggestion on >>> what already exists in config_mpc85xx.h: >>> #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ >>> defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) >>> #define CONFIG_E5500 >>> #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ >>> #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ >>> #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 >>> #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ >>> #ifdef CONFIG_SYS_FSL_DDR4 >>> #define CONFIG_SYS_FSL_DDRC_GEN4 >>> #endif >>> #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) >>> #define CONFIG_MAX_CPUS 4 >>> #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) >>> #define CONFIG_MAX_CPUS 2 >>> #endif >>> #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 >>> #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } >>> #define CONFIG_SYS_FSL_NUM_LAWS 16 >>> #define CONFIG_SYS_FSL_SRDS_1 >>> #define CONFIG_SYS_FSL_SEC_COMPAT 5 >>> #define CONFIG_SYS_NUM_FMAN 1 >>> #define CONFIG_SYS_NUM_FM1_DTSEC 5 >>> .... >>> etc. >>> >>> Seems like a good place to put another CPU constant. >>> >> >> Yeah. We have been moving things out of header files and into Kconfig. I >> would avoid adding new macro to header files. > > OK, I am happy if don't have to manually select the constant. > >> >>> Anyhow, that patch stands of its own I think. Where to put all constants >>> is another patch for NXP. >> >> Yes, we can add another patch to define this option for SoCs. > > :) > Jocke, Where are we on this patch? Do we have any patch to define PEX_CCB_DIV to activate this new code? York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2018-02-27 19:30 ` York Sun @ 2018-02-27 19:52 ` Joakim Tjernlund 2018-02-27 19:54 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2018-02-27 19:52 UTC (permalink / raw) To: u-boot On Tue, 2018-02-27 at 19:30 +0000, York Sun wrote: > > On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: > > On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: > > > > > > > > > On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: > > > > > > > > > > On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > > > > > > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > > > > > > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > > > > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > > > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > > > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > > > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > > > > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > > > > > > > type of CPU/platform. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > > > > > > > --- > > > > > > > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > > > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > > > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > > > > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > > > > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > > > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > > > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > > > > > > > + ((gd->bus_clk / 1000000) * > > > > > > > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > > > > > > > +#endif > > > > > > > > > > > > I took another look at this patch. Would it be appropriate to alway > > > write to this register with correct clock? > > > > I sure hope so, the docs I have only mentions this reg having a default value which > > needs to be changed in most cases I guess. > > > > > > > > > > > > > > > > > > > > > > block_rev = in_be32(&pci->block_rev1); > > > > > > > > > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > > > > > > > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > > > > > > > > > > > > > > > > > Ping? > > > > > > > > > > > > > > > > > > > > Jocke > > > > > > > > > > > > > > > > > > > > > > > > > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". > > > > > > > > > > > > > > > > Right, I seem to have forgotten to comment on that ... > > > > > > > > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > > > > > > > > it makes perfect sense to just #define it. > > > > > > > > > > > > > > > > > > > > > > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. > > > > > > > Would it be better if you use a Kconfig option and select the divisor by > > > > > > > SoC? > > > > > > > > > > > > No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define > > > > > > in relevant PPC CPU, like in config_mpc85xx.h > > > > > > > > > > > > > > > > So what should be in Kconfig, and what shouldn't? This is per SoC macro. > > > > > Sounds like CONFIG_SYS_ in old days. > > > > > > > > I must confess, I am not up to date with Kconfig stuff. I based my suggestion on > > > > what already exists in config_mpc85xx.h: > > > > #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ > > > > defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > > > #define CONFIG_E5500 > > > > #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ > > > > #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ > > > > #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 > > > > #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ > > > > #ifdef CONFIG_SYS_FSL_DDR4 > > > > #define CONFIG_SYS_FSL_DDRC_GEN4 > > > > #endif > > > > #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) > > > > #define CONFIG_MAX_CPUS 4 > > > > #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > > > #define CONFIG_MAX_CPUS 2 > > > > #endif > > > > #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 > > > > #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } > > > > #define CONFIG_SYS_FSL_NUM_LAWS 16 > > > > #define CONFIG_SYS_FSL_SRDS_1 > > > > #define CONFIG_SYS_FSL_SEC_COMPAT 5 > > > > #define CONFIG_SYS_NUM_FMAN 1 > > > > #define CONFIG_SYS_NUM_FM1_DTSEC 5 > > > > .... > > > > etc. > > > > > > > > Seems like a good place to put another CPU constant. > > > > > > > > > > Yeah. We have been moving things out of header files and into Kconfig. I > > > would avoid adding new macro to header files. > > > > OK, I am happy if don't have to manually select the constant. > > > > > > > > > Anyhow, that patch stands of its own I think. Where to put all constants > > > > is another patch for NXP. > > > > > > Yes, we can add another patch to define this option for SoCs. > > > > :) > > > > Jocke, > > Where are we on this patch? Do we have any patch to define PEX_CCB_DIV > to activate this new code? I think we are at me thinking we should add my simple patch: +#ifdef PEX_CCB_DIV + /* Configure the PCIE controller core clock ratio */ + pci_hose_write_config_dword(hose, dev, 0x440,> + ((gd->bus_clk / 1000000) * + (16 / PEX_CCB_DIV)) / 333); +#endif and then NXP find a suitable place to add PEX_CCB_DIV #define for each CPU/SOC. I am not working on such a patch anyhow. Jocke ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2018-02-27 19:52 ` Joakim Tjernlund @ 2018-02-27 19:54 ` York Sun 2018-08-02 22:38 ` Joakim Tjernlund 0 siblings, 1 reply; 21+ messages in thread From: York Sun @ 2018-02-27 19:54 UTC (permalink / raw) To: u-boot On 02/27/2018 11:52 AM, Joakim Tjernlund wrote: > On Tue, 2018-02-27 at 19:30 +0000, York Sun wrote: >> >> On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: >>> On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: >>>> >>>> >>>> On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: >>>>> On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: >>>>>> >>>>>> On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: >>>>>>> On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: >>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>> >>>>>>>> >>>>>>>> On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: >>>>>>>>> On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: >>>>>>>>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: >>>>>>>>>>> On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: >>>>>>>>>>>> Most FSL PCIe controllers expects 333 MHz PCI reference clock. >>>>>>>>>>>> This clock is derived from the CCB but in many cases the ref. >>>>>>>>>>>> clock is not 333 MHz and a divisor needs to be configured. >>>>>>>>>>>> >>>>>>>>>>>> This adds PEX_CCB_DIV #define which can be defined for each >>>>>>>>>>>> type of CPU/platform. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>>>>>>>>>>> --- >>>>>>>>>>>> drivers/pci/fsl_pci_init.c | 6 ++++++ >>>>>>>>>>>> 1 file changed, 6 insertions(+) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> index 52792dcd59..4d00b3f26c 100644 >>>>>>>>>>>> --- a/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> +++ b/drivers/pci/fsl_pci_init.c >>>>>>>>>>>> @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) >>>>>>>>>>>> >>>>>>>>>>>> pci_setup_indirect(hose, cfg_addr, cfg_data); >>>>>>>>>>>> >>>>>>>>>>>> +#ifdef PEX_CCB_DIV >>>>>>>>>>>> + /* Configure the PCIE controller core clock ratio */ >>>>>>>>>>>> + pci_hose_write_config_dword(hose, dev, 0x440, >>>>>>>>>>>> + ((gd->bus_clk / 1000000) * >>>>>>>>>>>> + (16 / PEX_CCB_DIV)) / 333); >>>>>>>>>>>> +#endif >>>> >>>> >>>> >>>> I took another look at this patch. Would it be appropriate to alway >>>> write to this register with correct clock? >>> >>> I sure hope so, the docs I have only mentions this reg having a default value which >>> needs to be changed in most cases I guess. >>> >>>> >>>> >>>> >>>>>>>>>>>> block_rev = in_be32(&pci->block_rev1); >>>>>>>>>>>> if (PEX_IP_BLK_REV_2_2 <= block_rev) { >>>>>>>>>>>> pi = &pci->pit[2]; /* 0xDC0 */ >>>>>>>>>>> >>>>>>>>>>> Ping? >>>>>>>>>>> >>>>>>>>>>> Jocke >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I believe Mingkai's last comment was "to add the PCIe clock in gd". >>>>>>>>> >>>>>>>>> Right, I seem to have forgotten to comment on that ... >>>>>>>>> Why add that complexity/bloat? This is a constant defined by the CPU/SOC so >>>>>>>>> it makes perfect sense to just #define it. >>>>>>>>> >>>>>>>> >>>>>>>> I am leaning to agree with you. The clock is either CCB clock, or CCB/2. >>>>>>>> Would it be better if you use a Kconfig option and select the divisor by >>>>>>>> SoC? >>>>>>> >>>>>>> No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define >>>>>>> in relevant PPC CPU, like in config_mpc85xx.h >>>>>>> >>>>>> >>>>>> So what should be in Kconfig, and what shouldn't? This is per SoC macro. >>>>>> Sounds like CONFIG_SYS_ in old days. >>>>> >>>>> I must confess, I am not up to date with Kconfig stuff. I based my suggestion on >>>>> what already exists in config_mpc85xx.h: >>>>> #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ >>>>> defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) >>>>> #define CONFIG_E5500 >>>>> #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ >>>>> #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ >>>>> #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 >>>>> #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ >>>>> #ifdef CONFIG_SYS_FSL_DDR4 >>>>> #define CONFIG_SYS_FSL_DDRC_GEN4 >>>>> #endif >>>>> #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) >>>>> #define CONFIG_MAX_CPUS 4 >>>>> #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) >>>>> #define CONFIG_MAX_CPUS 2 >>>>> #endif >>>>> #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 >>>>> #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } >>>>> #define CONFIG_SYS_FSL_NUM_LAWS 16 >>>>> #define CONFIG_SYS_FSL_SRDS_1 >>>>> #define CONFIG_SYS_FSL_SEC_COMPAT 5 >>>>> #define CONFIG_SYS_NUM_FMAN 1 >>>>> #define CONFIG_SYS_NUM_FM1_DTSEC 5 >>>>> .... >>>>> etc. >>>>> >>>>> Seems like a good place to put another CPU constant. >>>>> >>>> >>>> Yeah. We have been moving things out of header files and into Kconfig. I >>>> would avoid adding new macro to header files. >>> >>> OK, I am happy if don't have to manually select the constant. >>> >>>> >>>>> Anyhow, that patch stands of its own I think. Where to put all constants >>>>> is another patch for NXP. >>>> >>>> Yes, we can add another patch to define this option for SoCs. >>> >>> :) >>> >> >> Jocke, >> >> Where are we on this patch? Do we have any patch to define PEX_CCB_DIV >> to activate this new code? > > I think we are at me thinking we should add my simple patch: > +#ifdef PEX_CCB_DIV > + /* Configure the PCIE controller core clock ratio */ > + pci_hose_write_config_dword(hose, dev, 0x440,> > + ((gd->bus_clk / 1000000) * > + (16 / PEX_CCB_DIV)) / 333); > +#endif > and then NXP find a suitable place to add PEX_CCB_DIV #define for each CPU/SOC. > I am not working on such a patch anyhow. > OK. I tend to accept this patch and have Mingkai to follow up on defining PEX_CCB_DIV for SoCs in need. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2018-02-27 19:54 ` York Sun @ 2018-08-02 22:38 ` Joakim Tjernlund 2018-08-02 22:43 ` York Sun 0 siblings, 1 reply; 21+ messages in thread From: Joakim Tjernlund @ 2018-08-02 22:38 UTC (permalink / raw) To: u-boot York, did this go anywhere? Jocke On Tue, 2018-02-27 at 19:54 +0000, York Sun wrote: > > On 02/27/2018 11:52 AM, Joakim Tjernlund wrote: > > On Tue, 2018-02-27 at 19:30 +0000, York Sun wrote: > > > > > > On 11/21/2017 10:20 AM, Joakim Tjernlund wrote: > > > > On Tue, 2017-11-21 at 18:04 +0000, York Sun wrote: > > > > > > > > > > > > > > > On 11/21/2017 09:52 AM, Joakim Tjernlund wrote: > > > > > > On Tue, 2017-11-21 at 17:45 +0000, York Sun wrote: > > > > > > > > > > > > > > On 11/21/2017 09:41 AM, Joakim Tjernlund wrote: > > > > > > > > On Tue, 2017-11-21 at 17:32 +0000, York Sun wrote: > > > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:29 AM, Joakim Tjernlund wrote: > > > > > > > > > > On Tue, 2017-11-21 at 17:23 +0000, York Sun wrote: > > > > > > > > > > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 11/21/2017 09:18 AM, Joakim Tjernlund wrote: > > > > > > > > > > > > On Tue, 2017-09-12 at 19:56 +0200, Joakim Tjernlund wrote: > > > > > > > > > > > > > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > > > > > > > > > > > > > This clock is derived from the CCB but in many cases the ref. > > > > > > > > > > > > > clock is not 333 MHz and a divisor needs to be configured. > > > > > > > > > > > > > > > > > > > > > > > > > > This adds PEX_CCB_DIV #define which can be defined for each > > > > > > > > > > > > > type of CPU/platform. > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > > > > > > > > > > --- > > > > > > > > > > > > > drivers/pci/fsl_pci_init.c | 6 ++++++ > > > > > > > > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > > > index 52792dcd59..4d00b3f26c 100644 > > > > > > > > > > > > > --- a/drivers/pci/fsl_pci_init.c > > > > > > > > > > > > > +++ b/drivers/pci/fsl_pci_init.c > > > > > > > > > > > > > @@ -322,6 +322,12 @@ void fsl_pci_init(struct pci_controller *hose, struct fsl_pci_info *pci_info) > > > > > > > > > > > > > > > > > > > > > > > > > > pci_setup_indirect(hose, cfg_addr, cfg_data); > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef PEX_CCB_DIV > > > > > > > > > > > > > + /* Configure the PCIE controller core clock ratio */ > > > > > > > > > > > > > + pci_hose_write_config_dword(hose, dev, 0x440, > > > > > > > > > > > > > + ((gd->bus_clk / 1000000) * > > > > > > > > > > > > > + (16 / PEX_CCB_DIV)) / 333); > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > > > > > > > I took another look at this patch. Would it be appropriate to alway > > > > > write to this register with correct clock? > > > > > > > > I sure hope so, the docs I have only mentions this reg having a default value which > > > > needs to be changed in most cases I guess. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > block_rev = in_be32(&pci->block_rev1); > > > > > > > > > > > > > if (PEX_IP_BLK_REV_2_2 <= block_rev) { > > > > > > > > > > > > > pi = &pci->pit[2]; /* 0xDC0 */ > > > > > > > > > > > > > > > > > > > > > > > > Ping? > > > > > > > > > > > > > > > > > > > > > > > > Jocke > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I believe Mingkai's last comment was "to add the PCIe clock in gd". > > > > > > > > > > > > > > > > > > > > Right, I seem to have forgotten to comment on that ... > > > > > > > > > > Why add that complexity/bloat? This is a constant defined by the CPU/SOC so > > > > > > > > > > it makes perfect sense to just #define it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am leaning to agree with you. The clock is either CCB clock, or CCB/2. > > > > > > > > > Would it be better if you use a Kconfig option and select the divisor by > > > > > > > > > SoC? > > > > > > > > > > > > > > > > No, this is not something that needs Kconfig, just add the PEX_CCB_DIV #define > > > > > > > > in relevant PPC CPU, like in config_mpc85xx.h > > > > > > > > > > > > > > > > > > > > > > So what should be in Kconfig, and what shouldn't? This is per SoC macro. > > > > > > > Sounds like CONFIG_SYS_ in old days. > > > > > > > > > > > > I must confess, I am not up to date with Kconfig stuff. I based my suggestion on > > > > > > what already exists in config_mpc85xx.h: > > > > > > #elif defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) ||\ > > > > > > defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > > > > > #define CONFIG_E5500 > > > > > > #define CONFIG_FSL_CORENET /* Freescale CoreNet platform */ > > > > > > #define CONFIG_SYS_FSL_QORIQ_CHASSIS2 /* Freescale Chassis generation 2 */ > > > > > > #define CONFIG_SYS_FSL_CORES_PER_CLUSTER 1 > > > > > > #define CONFIG_SYS_FSL_QMAN_V3 /* QMAN version 3 */ > > > > > > #ifdef CONFIG_SYS_FSL_DDR4 > > > > > > #define CONFIG_SYS_FSL_DDRC_GEN4 > > > > > > #endif > > > > > > #if defined(CONFIG_PPC_T1040) || defined(CONFIG_PPC_T1042) > > > > > > #define CONFIG_MAX_CPUS 4 > > > > > > #elif defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) > > > > > > #define CONFIG_MAX_CPUS 2 > > > > > > #endif > > > > > > #define CONFIG_SYS_FSL_NUM_CC_PLLS 2 > > > > > > #define CONFIG_SYS_FSL_CLUSTER_CLOCKS { 1, 1, 1, 1 } > > > > > > #define CONFIG_SYS_FSL_NUM_LAWS 16 > > > > > > #define CONFIG_SYS_FSL_SRDS_1 > > > > > > #define CONFIG_SYS_FSL_SEC_COMPAT 5 > > > > > > #define CONFIG_SYS_NUM_FMAN 1 > > > > > > #define CONFIG_SYS_NUM_FM1_DTSEC 5 > > > > > > .... > > > > > > etc. > > > > > > > > > > > > Seems like a good place to put another CPU constant. > > > > > > > > > > > > > > > > Yeah. We have been moving things out of header files and into Kconfig. I > > > > > would avoid adding new macro to header files. > > > > > > > > OK, I am happy if don't have to manually select the constant. > > > > > > > > > > > > > > > Anyhow, that patch stands of its own I think. Where to put all constants > > > > > > is another patch for NXP. > > > > > > > > > > Yes, we can add another patch to define this option for SoCs. > > > > > > > > :) > > > > > > > > > > Jocke, > > > > > > Where are we on this patch? Do we have any patch to define PEX_CCB_DIV > > > to activate this new code? > > > > I think we are at me thinking we should add my simple patch: > > +#ifdef PEX_CCB_DIV > > + /* Configure the PCIE controller core clock ratio */ > > + pci_hose_write_config_dword(hose, dev, 0x440,> > > + ((gd->bus_clk / 1000000) * > > + (16 / PEX_CCB_DIV)) / 333); > > +#endif > > and then NXP find a suitable place to add PEX_CCB_DIV #define for each CPU/SOC. > > I am not working on such a patch anyhow. > > > > OK. I tend to accept this patch and have Mingkai to follow up on > defining PEX_CCB_DIV for SoCs in need. > > York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2018-08-02 22:38 ` Joakim Tjernlund @ 2018-08-02 22:43 ` York Sun 0 siblings, 0 replies; 21+ messages in thread From: York Sun @ 2018-08-02 22:43 UTC (permalink / raw) To: u-boot On 08/02/2018 03:38 PM, Joakim Tjernlund wrote: > York, did this go anywhere? No, I didn't hear from Mingkai. I am OK with this patch and will merge it. York ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio 2017-09-12 17:56 [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio Joakim Tjernlund 2017-09-14 21:15 ` York Sun 2017-11-21 17:18 ` Joakim Tjernlund @ 2018-08-13 16:29 ` York Sun 2 siblings, 0 replies; 21+ messages in thread From: York Sun @ 2018-08-13 16:29 UTC (permalink / raw) To: u-boot On 09/12/2017 10:56 AM, Joakim Tjernlund wrote: > Most FSL PCIe controllers expects 333 MHz PCI reference clock. > This clock is derived from the CCB but in many cases the ref. > clock is not 333 MHz and a divisor needs to be configured. > > This adds PEX_CCB_DIV #define which can be defined for each > type of CPU/platform. > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- Applied to fsl-qoriq master, awaiting upstream. Thanks. York ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-08-13 16:29 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-12 17:56 [U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio Joakim Tjernlund 2017-09-14 21:15 ` York Sun 2017-09-15 6:14 ` Mingkai Hu 2017-11-21 17:18 ` Joakim Tjernlund 2017-11-21 17:23 ` York Sun 2017-11-21 17:29 ` Joakim Tjernlund 2017-11-21 17:32 ` York Sun 2017-11-21 17:41 ` Joakim Tjernlund 2017-11-21 17:45 ` York Sun 2017-11-21 17:51 ` Joakim Tjernlund 2017-11-21 18:04 ` York Sun 2017-11-21 18:20 ` Joakim Tjernlund 2017-11-21 18:35 ` York Sun 2017-11-21 18:39 ` Joakim Tjernlund 2017-11-21 18:41 ` York Sun 2018-02-27 19:30 ` York Sun 2018-02-27 19:52 ` Joakim Tjernlund 2018-02-27 19:54 ` York Sun 2018-08-02 22:38 ` Joakim Tjernlund 2018-08-02 22:43 ` York Sun 2018-08-13 16:29 ` York Sun
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.