[U-Boot] [PATCH] FSL PCI: Configure PCIe reference ratio
York Sun
york.sun at nxp.com
Tue Feb 27 19:54:15 UTC 2018
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 at 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
More information about the U-Boot
mailing list