| Message ID | 1476347589-5578-5-git-send-email-clsee@altera.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
On 10/13/2016 10:33 AM, Chin Liang See wrote: > Separate the Clock Manager to support both GEN5 SoC and > Stratix 10 SoC. > > Signed-off-by: Chin Liang See <clsee@altera.com> > Cc: Marek Vasut <marex@denx.de> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Cc: Ley Foon Tan <lftan@altera.com> > Cc: Tien Fong Chee <tfchee@altera.com> > --- > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach-socfpga/clock_manager.c > index aa71636..0d67b3c 100644 > --- a/arch/arm/mach-socfpga/clock_manager.c > +++ b/arch/arm/mach-socfpga/clock_manager.c > @@ -10,6 +10,7 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > static const struct socfpga_clock_manager *clock_manager_base = > (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; > > @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) > > return clock; > } > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > unsigned int cm_get_mmc_controller_clk_hz(void) > { > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > uint32_t reg, clock = 0; > > /* identify the source of MMC clock */ > @@ -475,8 +478,12 @@ unsigned int cm_get_mmc_controller_clk_hz(void) > /* further divide by 4 as we have fixed divider at wrapper */ > clock /= 4; > return clock; > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > + return 25000000; Is this always gonna be the case or is this S10VP specific ? > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > } > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > unsigned int cm_get_qspi_controller_clk_hz(void) > { > uint32_t reg, clock = 0; > @@ -556,3 +563,4 @@ U_BOOT_CMD( > "display clocks", > "" Why does the clock display not work on S10 ? Are some functions missing? Maybe we should split the clock manager into common part and then gen5 and gen10 specific parts ? > ); > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >
On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > Separate the Clock Manager to support both GEN5 SoC and > > Stratix 10 SoC. > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > Cc: Ley Foon Tan <lftan@altera.com> > > Cc: Tien Fong Chee <tfchee@altera.com> > > --- > > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach- > > socfpga/clock_manager.c > > index aa71636..0d67b3c 100644 > > --- a/arch/arm/mach-socfpga/clock_manager.c > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > @@ -10,6 +10,7 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > static const struct socfpga_clock_manager *clock_manager_base = > > (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; > > > > @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) > > > > return clock; > > } > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > unsigned int cm_get_mmc_controller_clk_hz(void) > > { > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > uint32_t reg, clock = 0; > > > > /* identify the source of MMC clock */ > > @@ -475,8 +478,12 @@ unsigned int > > cm_get_mmc_controller_clk_hz(void) > > /* further divide by 4 as we have fixed divider at wrapper */ > > clock /= 4; > > return clock; > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > + return 25000000; > Is this always gonna be the case or is this S10VP specific ? > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > } > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > unsigned int cm_get_qspi_controller_clk_hz(void) > > { > > uint32_t reg, clock = 0; > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > "display clocks", > > "" > Why does the clock display not work on S10 ? Are some functions > missing? Not for SOCVP. But will be added in later stage when testing against emulation > > Maybe we should split the clock manager into common part and then > gen5 > and gen10 specific parts ? Ok, we can do that as initially we were worried too many files created within mach-socfpga. Thanks Chin Liang > > > > > ); > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > -- > Best regards, > Marek Vasut > > ________________________________ > > Confidentiality Notice. > This message may contain information that is confidential or > otherwise protected from disclosure. If you are not the intended > recipient, you are hereby notified that any use, disclosure, > dissemination, distribution, or copying of this message, or any > attachments, is strictly prohibited. If you have received this > message in error, please advise the sender by reply e-mail, and > delete the message and any attachments. Thank you.
On 10/17/2016 03:32 PM, See, Chin Liang wrote: > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: >> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>> >>> Separate the Clock Manager to support both GEN5 SoC and >>> Stratix 10 SoC. >>> >>> Signed-off-by: Chin Liang See <clsee@altera.com> >>> Cc: Marek Vasut <marex@denx.de> >>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>> Cc: Ley Foon Tan <lftan@altera.com> >>> Cc: Tien Fong Chee <tfchee@altera.com> >>> --- >>> arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach- >>> socfpga/clock_manager.c >>> index aa71636..0d67b3c 100644 >>> --- a/arch/arm/mach-socfpga/clock_manager.c >>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>> @@ -10,6 +10,7 @@ >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>> static const struct socfpga_clock_manager *clock_manager_base = >>> (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; >>> >>> @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) >>> >>> return clock; >>> } >>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>> >>> unsigned int cm_get_mmc_controller_clk_hz(void) >>> { >>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>> uint32_t reg, clock = 0; >>> >>> /* identify the source of MMC clock */ >>> @@ -475,8 +478,12 @@ unsigned int >>> cm_get_mmc_controller_clk_hz(void) >>> /* further divide by 4 as we have fixed divider at wrapper */ >>> clock /= 4; >>> return clock; >>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>> + return 25000000; >> Is this always gonna be the case or is this S10VP specific ? >> >>> >>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>> } >>> >>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>> unsigned int cm_get_qspi_controller_clk_hz(void) >>> { >>> uint32_t reg, clock = 0; >>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>> "display clocks", >>> "" >> Why does the clock display not work on S10 ? Are some functions >> missing? > > Not for SOCVP. But will be added in later stage when testing against > emulation How hard would it be to add this missing functionality now ? >> Maybe we should split the clock manager into common part and then >> gen5 >> and gen10 specific parts ? > > Ok, we can do that as initially we were worried too many files created > within mach-socfpga. It's probably better than polluting the clock code with ifdefs. [...]
On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: > On 10/17/2016 03:32 PM, See, Chin Liang wrote: > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > > > > > > > > > Separate the Clock Manager to support both GEN5 SoC and > > > > Stratix 10 SoC. > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > Cc: Marek Vasut <marex@denx.de> > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > Cc: Ley Foon Tan <lftan@altera.com> > > > > Cc: Tien Fong Chee <tfchee@altera.com> > > > > --- > > > > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager.c > > > > b/arch/arm/mach- > > > > socfpga/clock_manager.c > > > > index aa71636..0d67b3c 100644 > > > > --- a/arch/arm/mach-socfpga/clock_manager.c > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > > > @@ -10,6 +10,7 @@ > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > static const struct socfpga_clock_manager *clock_manager_base > > > > = > > > > (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; > > > > > > > > @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) > > > > > > > > return clock; > > > > } > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > unsigned int cm_get_mmc_controller_clk_hz(void) > > > > { > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > uint32_t reg, clock = 0; > > > > > > > > /* identify the source of MMC clock */ > > > > @@ -475,8 +478,12 @@ unsigned int > > > > cm_get_mmc_controller_clk_hz(void) > > > > /* further divide by 4 as we have fixed divider at > > > > wrapper */ > > > > clock /= 4; > > > > return clock; > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > > > + return 25000000; > > > Is this always gonna be the case or is this S10VP specific ? > > > > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > } > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > unsigned int cm_get_qspi_controller_clk_hz(void) > > > > { > > > > uint32_t reg, clock = 0; > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > > > "display clocks", > > > > "" > > > Why does the clock display not work on S10 ? Are some functions > > > missing? > > Not for SOCVP. But will be added in later stage when testing > > against > > emulation > How hard would it be to add this missing functionality now ? > That will take weeks as that need to be validated as whole in emulation platform. > > > > > > > > Maybe we should split the clock manager into common part and then > > > gen5 > > > and gen10 specific parts ? > > Ok, we can do that as initially we were worried too many files > > created > > within mach-socfpga. > It's probably better than polluting the clock code with ifdefs. > Ok, we have an consensus then Thanks Chin Liang > [...] > >
On 10/17/2016 05:07 PM, Chin Liang See wrote: > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: >> On 10/17/2016 03:32 PM, See, Chin Liang wrote: >>> >>> On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: >>>> >>>> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>>>> >>>>> >>>>> Separate the Clock Manager to support both GEN5 SoC and >>>>> Stratix 10 SoC. >>>>> >>>>> Signed-off-by: Chin Liang See <clsee@altera.com> >>>>> Cc: Marek Vasut <marex@denx.de> >>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>>>> Cc: Ley Foon Tan <lftan@altera.com> >>>>> Cc: Tien Fong Chee <tfchee@altera.com> >>>>> --- >>>>> arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/arch/arm/mach-socfpga/clock_manager.c >>>>> b/arch/arm/mach- >>>>> socfpga/clock_manager.c >>>>> index aa71636..0d67b3c 100644 >>>>> --- a/arch/arm/mach-socfpga/clock_manager.c >>>>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>>>> @@ -10,6 +10,7 @@ >>>>> >>>>> DECLARE_GLOBAL_DATA_PTR; >>>>> >>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>> static const struct socfpga_clock_manager *clock_manager_base >>>>> = >>>>> (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; >>>>> >>>>> @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) >>>>> >>>>> return clock; >>>>> } >>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>> >>>>> unsigned int cm_get_mmc_controller_clk_hz(void) >>>>> { >>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>> uint32_t reg, clock = 0; >>>>> >>>>> /* identify the source of MMC clock */ >>>>> @@ -475,8 +478,12 @@ unsigned int >>>>> cm_get_mmc_controller_clk_hz(void) >>>>> /* further divide by 4 as we have fixed divider at >>>>> wrapper */ >>>>> clock /= 4; >>>>> return clock; >>>>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>>>> + return 25000000; >>>> Is this always gonna be the case or is this S10VP specific ? >>>> >>>>> >>>>> >>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>> } >>>>> >>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>> unsigned int cm_get_qspi_controller_clk_hz(void) >>>>> { >>>>> uint32_t reg, clock = 0; >>>>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>>>> "display clocks", >>>>> "" >>>> Why does the clock display not work on S10 ? Are some functions >>>> missing? >>> Not for SOCVP. But will be added in later stage when testing >>> against >>> emulation >> How hard would it be to add this missing functionality now ? >> > > That will take weeks as that need to be validated as whole in emulation > platform. You mean printing a few clock information based on some values from registers would take weeks ? Why ? >>>> Maybe we should split the clock manager into common part and then >>>> gen5 >>>> and gen10 specific parts ? >>> Ok, we can do that as initially we were worried too many files >>> created >>> within mach-socfpga. >> It's probably better than polluting the clock code with ifdefs. >> > > Ok, we have an consensus then OK > Thanks > Chin Liang > > >> [...] >> >>
On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: > On 10/17/2016 05:07 PM, Chin Liang See wrote: > > > > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: > > > > > > On 10/17/2016 03:32 PM, See, Chin Liang wrote: > > > > > > > > > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > Separate the Clock Manager to support both GEN5 SoC and > > > > > > Stratix 10 SoC. > > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > Cc: Ley Foon Tan <lftan@altera.com> > > > > > > Cc: Tien Fong Chee <tfchee@altera.com> > > > > > > --- > > > > > > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager.c > > > > > > b/arch/arm/mach- > > > > > > socfpga/clock_manager.c > > > > > > index aa71636..0d67b3c 100644 > > > > > > --- a/arch/arm/mach-socfpga/clock_manager.c > > > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > > > > > @@ -10,6 +10,7 @@ > > > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > static const struct socfpga_clock_manager > > > > > > *clock_manager_base > > > > > > = > > > > > > (struct socfpga_clock_manager > > > > > > *)SOCFPGA_CLKMGR_ADDRESS; > > > > > > > > > > > > @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) > > > > > > > > > > > > return clock; > > > > > > } > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > > > unsigned int cm_get_mmc_controller_clk_hz(void) > > > > > > { > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > > > /* identify the source of MMC clock */ > > > > > > @@ -475,8 +478,12 @@ unsigned int > > > > > > cm_get_mmc_controller_clk_hz(void) > > > > > > /* further divide by 4 as we have fixed divider at > > > > > > wrapper */ > > > > > > clock /= 4; > > > > > > return clock; > > > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > > > > > + return 25000000; > > > > > Is this always gonna be the case or is this S10VP specific ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > } > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > unsigned int cm_get_qspi_controller_clk_hz(void) > > > > > > { > > > > > > uint32_t reg, clock = 0; > > > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > > > > > "display clocks", > > > > > > "" > > > > > Why does the clock display not work on S10 ? Are some > > > > > functions > > > > > missing? > > > > Not for SOCVP. But will be added in later stage when testing > > > > against > > > > emulation > > > How hard would it be to add this missing functionality now ? > > > > > That will take weeks as that need to be validated as whole in > > emulation > > platform. > You mean printing a few clock information based on some values from > registers would take weeks ? Why ? > Oh actually I am referring all the managers code such as full Clock Manager, Reset Manager ... plus testing. Testing is the part take some significant time especially slow when come to emulation. Thanks Chin Liang [..]
On 10/17/2016 05:28 PM, Chin Liang See wrote: > On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: >> On 10/17/2016 05:07 PM, Chin Liang See wrote: >>> >>> On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: >>>> >>>> On 10/17/2016 03:32 PM, See, Chin Liang wrote: >>>>> >>>>> >>>>> On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Separate the Clock Manager to support both GEN5 SoC and >>>>>>> Stratix 10 SoC. >>>>>>> >>>>>>> Signed-off-by: Chin Liang See <clsee@altera.com> >>>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>>>>>> Cc: Ley Foon Tan <lftan@altera.com> >>>>>>> Cc: Tien Fong Chee <tfchee@altera.com> >>>>>>> --- >>>>>>> arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ >>>>>>> 1 file changed, 8 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/mach-socfpga/clock_manager.c >>>>>>> b/arch/arm/mach- >>>>>>> socfpga/clock_manager.c >>>>>>> index aa71636..0d67b3c 100644 >>>>>>> --- a/arch/arm/mach-socfpga/clock_manager.c >>>>>>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>>>>>> @@ -10,6 +10,7 @@ >>>>>>> >>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>> >>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>> static const struct socfpga_clock_manager >>>>>>> *clock_manager_base >>>>>>> = >>>>>>> (struct socfpga_clock_manager >>>>>>> *)SOCFPGA_CLKMGR_ADDRESS; >>>>>>> >>>>>>> @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) >>>>>>> >>>>>>> return clock; >>>>>>> } >>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>> >>>>>>> unsigned int cm_get_mmc_controller_clk_hz(void) >>>>>>> { >>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>> uint32_t reg, clock = 0; >>>>>>> >>>>>>> /* identify the source of MMC clock */ >>>>>>> @@ -475,8 +478,12 @@ unsigned int >>>>>>> cm_get_mmc_controller_clk_hz(void) >>>>>>> /* further divide by 4 as we have fixed divider at >>>>>>> wrapper */ >>>>>>> clock /= 4; >>>>>>> return clock; >>>>>>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>>>>>> + return 25000000; >>>>>> Is this always gonna be the case or is this S10VP specific ? >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>> } >>>>>>> >>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>> unsigned int cm_get_qspi_controller_clk_hz(void) >>>>>>> { >>>>>>> uint32_t reg, clock = 0; >>>>>>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>>>>>> "display clocks", >>>>>>> "" >>>>>> Why does the clock display not work on S10 ? Are some >>>>>> functions >>>>>> missing? >>>>> Not for SOCVP. But will be added in later stage when testing >>>>> against >>>>> emulation >>>> How hard would it be to add this missing functionality now ? >>>> >>> That will take weeks as that need to be validated as whole in >>> emulation >>> platform. >> You mean printing a few clock information based on some values from >> registers would take weeks ? Why ? >> > > Oh actually I am referring all the managers code such as full Clock > Manager, Reset Manager ... plus testing. Testing is the part take some > significant time especially slow when come to emulation. Just use empty functions for the clock init code (since it's not needed on the socvp) and populate the clock reporting functions. That should be simple, right ?
On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: > On 10/17/2016 05:28 PM, Chin Liang See wrote: > > > > On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: > > > > > > On 10/17/2016 05:07 PM, Chin Liang See wrote: > > > > > > > > > > > > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 10/17/2016 03:32 PM, See, Chin Liang wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Separate the Clock Manager to support both GEN5 SoC and > > > > > > > > Stratix 10 SoC. > > > > > > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > > > Cc: Ley Foon Tan <lftan@altera.com> > > > > > > > > Cc: Tien Fong Chee <tfchee@altera.com> > > > > > > > > --- > > > > > > > > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > b/arch/arm/mach- > > > > > > > > socfpga/clock_manager.c > > > > > > > > index aa71636..0d67b3c 100644 > > > > > > > > --- a/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > @@ -10,6 +10,7 @@ > > > > > > > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > static const struct socfpga_clock_manager > > > > > > > > *clock_manager_base > > > > > > > > = > > > > > > > > (struct socfpga_clock_manager > > > > > > > > *)SOCFPGA_CLKMGR_ADDRESS; > > > > > > > > > > > > > > > > @@ -446,9 +447,11 @@ unsigned int > > > > > > > > cm_get_l4_sp_clk_hz(void) > > > > > > > > > > > > > > > > return clock; > > > > > > > > } > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > > > > > > > unsigned int cm_get_mmc_controller_clk_hz(void) > > > > > > > > { > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > > > > > > > /* identify the source of MMC clock */ > > > > > > > > @@ -475,8 +478,12 @@ unsigned int > > > > > > > > cm_get_mmc_controller_clk_hz(void) > > > > > > > > /* further divide by 4 as we have fixed divider > > > > > > > > at > > > > > > > > wrapper */ > > > > > > > > clock /= 4; > > > > > > > > return clock; > > > > > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > > > > > > > + return 25000000; > > > > > > > Is this always gonna be the case or is this S10VP > > > > > > > specific ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > } > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > unsigned int cm_get_qspi_controller_clk_hz(void) > > > > > > > > { > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > > > > > > > "display clocks", > > > > > > > > "" > > > > > > > Why does the clock display not work on S10 ? Are some > > > > > > > functions > > > > > > > missing? > > > > > > Not for SOCVP. But will be added in later stage when > > > > > > testing > > > > > > against > > > > > > emulation > > > > > How hard would it be to add this missing functionality now ? > > > > > > > > > That will take weeks as that need to be validated as whole in > > > > emulation > > > > platform. > > > You mean printing a few clock information based on some values > > > from > > > registers would take weeks ? Why ? > > > > > Oh actually I am referring all the managers code such as full Clock > > Manager, Reset Manager ... plus testing. Testing is the part take > > some > > significant time especially slow when come to emulation. > Just use empty functions for the clock init code (since it's not > needed > on the socvp) and populate the clock reporting functions. That should > be > simple, right ? Can be done but the value won't be meaningful as the register is uninitialzied. Unless we hardcode to a hard value which might not sound right. Thanks Chin Liang > > -- > Best regards, > Marek Vasut >
On 10/17/2016 05:59 PM, Chin Liang See wrote: > On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: >> On 10/17/2016 05:28 PM, Chin Liang See wrote: >>> >>> On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: >>>> >>>> On 10/17/2016 05:07 PM, Chin Liang See wrote: >>>>> >>>>> >>>>> On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 10/17/2016 03:32 PM, See, Chin Liang wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Separate the Clock Manager to support both GEN5 SoC and >>>>>>>>> Stratix 10 SoC. >>>>>>>>> >>>>>>>>> Signed-off-by: Chin Liang See <clsee@altera.com> >>>>>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>>>>>>>> Cc: Ley Foon Tan <lftan@altera.com> >>>>>>>>> Cc: Tien Fong Chee <tfchee@altera.com> >>>>>>>>> --- >>>>>>>>> arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ >>>>>>>>> 1 file changed, 8 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>> b/arch/arm/mach- >>>>>>>>> socfpga/clock_manager.c >>>>>>>>> index aa71636..0d67b3c 100644 >>>>>>>>> --- a/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>> @@ -10,6 +10,7 @@ >>>>>>>>> >>>>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>>>> >>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>> static const struct socfpga_clock_manager >>>>>>>>> *clock_manager_base >>>>>>>>> = >>>>>>>>> (struct socfpga_clock_manager >>>>>>>>> *)SOCFPGA_CLKMGR_ADDRESS; >>>>>>>>> >>>>>>>>> @@ -446,9 +447,11 @@ unsigned int >>>>>>>>> cm_get_l4_sp_clk_hz(void) >>>>>>>>> >>>>>>>>> return clock; >>>>>>>>> } >>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>> >>>>>>>>> unsigned int cm_get_mmc_controller_clk_hz(void) >>>>>>>>> { >>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>> >>>>>>>>> /* identify the source of MMC clock */ >>>>>>>>> @@ -475,8 +478,12 @@ unsigned int >>>>>>>>> cm_get_mmc_controller_clk_hz(void) >>>>>>>>> /* further divide by 4 as we have fixed divider >>>>>>>>> at >>>>>>>>> wrapper */ >>>>>>>>> clock /= 4; >>>>>>>>> return clock; >>>>>>>>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>>>>>>>> + return 25000000; >>>>>>>> Is this always gonna be the case or is this S10VP >>>>>>>> specific ? >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>> } >>>>>>>>> >>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>> unsigned int cm_get_qspi_controller_clk_hz(void) >>>>>>>>> { >>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>>>>>>>> "display clocks", >>>>>>>>> "" >>>>>>>> Why does the clock display not work on S10 ? Are some >>>>>>>> functions >>>>>>>> missing? >>>>>>> Not for SOCVP. But will be added in later stage when >>>>>>> testing >>>>>>> against >>>>>>> emulation >>>>>> How hard would it be to add this missing functionality now ? >>>>>> >>>>> That will take weeks as that need to be validated as whole in >>>>> emulation >>>>> platform. >>>> You mean printing a few clock information based on some values >>>> from >>>> registers would take weeks ? Why ? >>>> >>> Oh actually I am referring all the managers code such as full Clock >>> Manager, Reset Manager ... plus testing. Testing is the part take >>> some >>> significant time especially slow when come to emulation. >> Just use empty functions for the clock init code (since it's not >> needed >> on the socvp) and populate the clock reporting functions. That should >> be >> simple, right ? > > Can be done but the value won't be meaningful as the register is > uninitialzied. Unless we hardcode to a hard value which might not sound > right. Ha, I see. Is there some sane default for the SoCVP ?
On Sen, 2016-10-17 at 18:14 +0200, Marek Vasut wrote: > On 10/17/2016 05:59 PM, Chin Liang See wrote: > > > > On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: > > > > > > On 10/17/2016 05:28 PM, Chin Liang See wrote: > > > > > > > > > > > > On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 10/17/2016 05:07 PM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/17/2016 03:32 PM, See, Chin Liang wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Separate the Clock Manager to support both GEN5 SoC > > > > > > > > > > and > > > > > > > > > > Stratix 10 SoC. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > > > > > Cc: Ley Foon Tan <lftan@altera.com> > > > > > > > > > > Cc: Tien Fong Chee <tfchee@altera.com> > > > > > > > > > > --- > > > > > > > > > > arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ > > > > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > > > b/arch/arm/mach- > > > > > > > > > > socfpga/clock_manager.c > > > > > > > > > > index aa71636..0d67b3c 100644 > > > > > > > > > > --- a/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > > > @@ -10,6 +10,7 @@ > > > > > > > > > > > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > static const struct socfpga_clock_manager > > > > > > > > > > *clock_manager_base > > > > > > > > > > = > > > > > > > > > > (struct socfpga_clock_manager > > > > > > > > > > *)SOCFPGA_CLKMGR_ADDRESS; > > > > > > > > > > > > > > > > > > > > @@ -446,9 +447,11 @@ unsigned int > > > > > > > > > > cm_get_l4_sp_clk_hz(void) > > > > > > > > > > > > > > > > > > > > return clock; > > > > > > > > > > } > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > > > > > > > > > > > unsigned int cm_get_mmc_controller_clk_hz(void) > > > > > > > > > > { > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > > > > > > > > > > > /* identify the source of MMC clock */ > > > > > > > > > > @@ -475,8 +478,12 @@ unsigned int > > > > > > > > > > cm_get_mmc_controller_clk_hz(void) > > > > > > > > > > /* further divide by 4 as we have fixed > > > > > > > > > > divider > > > > > > > > > > at > > > > > > > > > > wrapper */ > > > > > > > > > > clock /= 4; > > > > > > > > > > return clock; > > > > > > > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > > > > > > > > > + return 25000000; > > > > > > > > > Is this always gonna be the case or is this S10VP > > > > > > > > > specific ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > unsigned int cm_get_qspi_controller_clk_hz(void) > > > > > > > > > > { > > > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > > > > > > > > > "display clocks", > > > > > > > > > > "" > > > > > > > > > Why does the clock display not work on S10 ? Are some > > > > > > > > > functions > > > > > > > > > missing? > > > > > > > > Not for SOCVP. But will be added in later stage when > > > > > > > > testing > > > > > > > > against > > > > > > > > emulation > > > > > > > How hard would it be to add this missing functionality > > > > > > > now ? > > > > > > > > > > > > > That will take weeks as that need to be validated as whole > > > > > > in > > > > > > emulation > > > > > > platform. > > > > > You mean printing a few clock information based on some > > > > > values > > > > > from > > > > > registers would take weeks ? Why ? > > > > > > > > > Oh actually I am referring all the managers code such as full > > > > Clock > > > > Manager, Reset Manager ... plus testing. Testing is the part > > > > take > > > > some > > > > significant time especially slow when come to emulation. > > > Just use empty functions for the clock init code (since it's not > > > needed > > > on the socvp) and populate the clock reporting functions. That > > > should > > > be > > > simple, right ? > > Can be done but the value won't be meaningful as the register is > > uninitialzied. Unless we hardcode to a hard value which might not > > sound > > right. > Ha, I see. Is there some sane default for the SoCVP ? > It will not meaningful as performance is same even with frequency change :) But definitely will address this in coming weeks as I am already working on the emulation. Thanks Chin Liang > -- > Best regards, > Marek Vasut >
On Sel, 2016-10-18 at 06:00 +0200, Marek Vasut wrote: > On 10/18/2016 05:22 AM, Chin Liang See wrote: > > > > On Sen, 2016-10-17 at 18:14 +0200, Marek Vasut wrote: > > > > > > On 10/17/2016 05:59 PM, Chin Liang See wrote: > > > > > > > > > > > > On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > On 10/17/2016 05:28 PM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/17/2016 05:07 PM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/17/2016 03:32 PM, See, Chin Liang wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Min, 2016-10-16 at 17:33 +0200, Marek Vasut > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 10/13/2016 10:33 AM, Chin Liang See wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Separate the Clock Manager to support both GEN5 > > > > > > > > > > > > SoC > > > > > > > > > > > > and > > > > > > > > > > > > Stratix 10 SoC. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com > > > > > > > > > > > > > > > > > > > > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > > > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com > > > > > > > > > > > > > > > > > > > > > > > > > Cc: Ley Foon Tan <lftan@altera.com> > > > > > > > > > > > > Cc: Tien Fong Chee <tfchee@altera.com> > > > > > > > > > > > > --- > > > > > > > > > > > > arch/arm/mach-socfpga/clock_manager.c | 8 > > > > > > > > > > > > ++++++++ > > > > > > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/arm/mach- > > > > > > > > > > > > socfpga/clock_manager.c > > > > > > > > > > > > b/arch/arm/mach- > > > > > > > > > > > > socfpga/clock_manager.c > > > > > > > > > > > > index aa71636..0d67b3c 100644 > > > > > > > > > > > > --- a/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > > > > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > > > > > > > > > > > @@ -10,6 +10,7 @@ > > > > > > > > > > > > > > > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > > > static const struct socfpga_clock_manager > > > > > > > > > > > > *clock_manager_base > > > > > > > > > > > > = > > > > > > > > > > > > (struct socfpga_clock_manager > > > > > > > > > > > > *)SOCFPGA_CLKMGR_ADDRESS; > > > > > > > > > > > > > > > > > > > > > > > > @@ -446,9 +447,11 @@ unsigned int > > > > > > > > > > > > cm_get_l4_sp_clk_hz(void) > > > > > > > > > > > > > > > > > > > > > > > > return clock; > > > > > > > > > > > > } > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > > > > > > > > > > > > > > > unsigned int > > > > > > > > > > > > cm_get_mmc_controller_clk_hz(void) > > > > > > > > > > > > { > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > > > > > > > > > > > > > > > /* identify the source of MMC clock */ > > > > > > > > > > > > @@ -475,8 +478,12 @@ unsigned int > > > > > > > > > > > > cm_get_mmc_controller_clk_hz(void) > > > > > > > > > > > > /* further divide by 4 as we have fixed > > > > > > > > > > > > divider > > > > > > > > > > > > at > > > > > > > > > > > > wrapper */ > > > > > > > > > > > > clock /= 4; > > > > > > > > > > > > return clock; > > > > > > > > > > > > +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) > > > > > > > > > > > > + return 25000000; > > > > > > > > > > > Is this always gonna be the case or is this S10VP > > > > > > > > > > > specific ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) > > > > > > > > > > > > unsigned int > > > > > > > > > > > > cm_get_qspi_controller_clk_hz(void) > > > > > > > > > > > > { > > > > > > > > > > > > uint32_t reg, clock = 0; > > > > > > > > > > > > @@ -556,3 +563,4 @@ U_BOOT_CMD( > > > > > > > > > > > > "display clocks", > > > > > > > > > > > > "" > > > > > > > > > > > Why does the clock display not work on S10 ? Are > > > > > > > > > > > some > > > > > > > > > > > functions > > > > > > > > > > > missing? > > > > > > > > > > Not for SOCVP. But will be added in later stage > > > > > > > > > > when > > > > > > > > > > testing > > > > > > > > > > against > > > > > > > > > > emulation > > > > > > > > > How hard would it be to add this missing > > > > > > > > > functionality > > > > > > > > > now ? > > > > > > > > > > > > > > > > > That will take weeks as that need to be validated as > > > > > > > > whole > > > > > > > > in > > > > > > > > emulation > > > > > > > > platform. > > > > > > > You mean printing a few clock information based on some > > > > > > > values > > > > > > > from > > > > > > > registers would take weeks ? Why ? > > > > > > > > > > > > > Oh actually I am referring all the managers code such as > > > > > > full > > > > > > Clock > > > > > > Manager, Reset Manager ... plus testing. Testing is the > > > > > > part > > > > > > take > > > > > > some > > > > > > significant time especially slow when come to emulation. > > > > > Just use empty functions for the clock init code (since it's > > > > > not > > > > > needed > > > > > on the socvp) and populate the clock reporting functions. > > > > > That > > > > > should > > > > > be > > > > > simple, right ? > > > > Can be done but the value won't be meaningful as the register > > > > is > > > > uninitialzied. Unless we hardcode to a hard value which might > > > > not > > > > sound > > > > right. > > > Ha, I see. Is there some sane default for the SoCVP ? > > > > > It will not meaningful as performance is same even with frequency > > change :) But definitely will address this in coming weeks as I am > > already working on the emulation. > I understand that, but I believe it is still better to provide some > sensible value rather than just ifdef the whole code out. I can hardcode some sensible value for that if good. Thanks Chin Liang >
On 10/18/2016 05:22 AM, Chin Liang See wrote: > On Sen, 2016-10-17 at 18:14 +0200, Marek Vasut wrote: >> On 10/17/2016 05:59 PM, Chin Liang See wrote: >>> >>> On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: >>>> >>>> On 10/17/2016 05:28 PM, Chin Liang See wrote: >>>>> >>>>> >>>>> On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 10/17/2016 05:07 PM, Chin Liang See wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 10/17/2016 03:32 PM, See, Chin Liang wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Min, 2016-10-16 at 17:33 +0200, Marek Vasut wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Separate the Clock Manager to support both GEN5 SoC >>>>>>>>>>> and >>>>>>>>>>> Stratix 10 SoC. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Chin Liang See <clsee@altera.com> >>>>>>>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>>>>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> >>>>>>>>>>> Cc: Ley Foon Tan <lftan@altera.com> >>>>>>>>>>> Cc: Tien Fong Chee <tfchee@altera.com> >>>>>>>>>>> --- >>>>>>>>>>> arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ >>>>>>>>>>> 1 file changed, 8 insertions(+) >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>>>> b/arch/arm/mach- >>>>>>>>>>> socfpga/clock_manager.c >>>>>>>>>>> index aa71636..0d67b3c 100644 >>>>>>>>>>> --- a/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>>>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>>>> @@ -10,6 +10,7 @@ >>>>>>>>>>> >>>>>>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>>>>>> >>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>> static const struct socfpga_clock_manager >>>>>>>>>>> *clock_manager_base >>>>>>>>>>> = >>>>>>>>>>> (struct socfpga_clock_manager >>>>>>>>>>> *)SOCFPGA_CLKMGR_ADDRESS; >>>>>>>>>>> >>>>>>>>>>> @@ -446,9 +447,11 @@ unsigned int >>>>>>>>>>> cm_get_l4_sp_clk_hz(void) >>>>>>>>>>> >>>>>>>>>>> return clock; >>>>>>>>>>> } >>>>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>>>> >>>>>>>>>>> unsigned int cm_get_mmc_controller_clk_hz(void) >>>>>>>>>>> { >>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>>>> >>>>>>>>>>> /* identify the source of MMC clock */ >>>>>>>>>>> @@ -475,8 +478,12 @@ unsigned int >>>>>>>>>>> cm_get_mmc_controller_clk_hz(void) >>>>>>>>>>> /* further divide by 4 as we have fixed >>>>>>>>>>> divider >>>>>>>>>>> at >>>>>>>>>>> wrapper */ >>>>>>>>>>> clock /= 4; >>>>>>>>>>> return clock; >>>>>>>>>>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>>>>>>>>>> + return 25000000; >>>>>>>>>> Is this always gonna be the case or is this S10VP >>>>>>>>>> specific ? >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>>>> } >>>>>>>>>>> >>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>> unsigned int cm_get_qspi_controller_clk_hz(void) >>>>>>>>>>> { >>>>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>>>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>>>>>>>>>> "display clocks", >>>>>>>>>>> "" >>>>>>>>>> Why does the clock display not work on S10 ? Are some >>>>>>>>>> functions >>>>>>>>>> missing? >>>>>>>>> Not for SOCVP. But will be added in later stage when >>>>>>>>> testing >>>>>>>>> against >>>>>>>>> emulation >>>>>>>> How hard would it be to add this missing functionality >>>>>>>> now ? >>>>>>>> >>>>>>> That will take weeks as that need to be validated as whole >>>>>>> in >>>>>>> emulation >>>>>>> platform. >>>>>> You mean printing a few clock information based on some >>>>>> values >>>>>> from >>>>>> registers would take weeks ? Why ? >>>>>> >>>>> Oh actually I am referring all the managers code such as full >>>>> Clock >>>>> Manager, Reset Manager ... plus testing. Testing is the part >>>>> take >>>>> some >>>>> significant time especially slow when come to emulation. >>>> Just use empty functions for the clock init code (since it's not >>>> needed >>>> on the socvp) and populate the clock reporting functions. That >>>> should >>>> be >>>> simple, right ? >>> Can be done but the value won't be meaningful as the register is >>> uninitialzied. Unless we hardcode to a hard value which might not >>> sound >>> right. >> Ha, I see. Is there some sane default for the SoCVP ? >> > > It will not meaningful as performance is same even with frequency > change :) But definitely will address this in coming weeks as I am > already working on the emulation. I understand that, but I believe it is still better to provide some sensible value rather than just ifdef the whole code out.
On 10/18/2016 05:34 AM, Chin Liang See wrote: > On Sel, 2016-10-18 at 06:00 +0200, Marek Vasut wrote: >> On 10/18/2016 05:22 AM, Chin Liang See wrote: >>> >>> On Sen, 2016-10-17 at 18:14 +0200, Marek Vasut wrote: >>>> >>>> On 10/17/2016 05:59 PM, Chin Liang See wrote: >>>>> >>>>> >>>>> On Sen, 2016-10-17 at 17:39 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 10/17/2016 05:28 PM, Chin Liang See wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Sen, 2016-10-17 at 17:20 +0200, Marek Vasut wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 10/17/2016 05:07 PM, Chin Liang See wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Sen, 2016-10-17 at 15:42 +0200, Marek Vasut wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 10/17/2016 03:32 PM, See, Chin Liang wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Min, 2016-10-16 at 17:33 +0200, Marek Vasut >>>>>>>>>>> wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On 10/13/2016 10:33 AM, Chin Liang See wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Separate the Clock Manager to support both GEN5 >>>>>>>>>>>>> SoC >>>>>>>>>>>>> and >>>>>>>>>>>>> Stratix 10 SoC. >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: Chin Liang See <clsee@altera.com >>>>>>>>>>>>>> >>>>>>>>>>>>> Cc: Marek Vasut <marex@denx.de> >>>>>>>>>>>>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com >>>>>>>>>>>>>> >>>>>>>>>>>>> Cc: Ley Foon Tan <lftan@altera.com> >>>>>>>>>>>>> Cc: Tien Fong Chee <tfchee@altera.com> >>>>>>>>>>>>> --- >>>>>>>>>>>>> arch/arm/mach-socfpga/clock_manager.c | 8 >>>>>>>>>>>>> ++++++++ >>>>>>>>>>>>> 1 file changed, 8 insertions(+) >>>>>>>>>>>>> >>>>>>>>>>>>> diff --git a/arch/arm/mach- >>>>>>>>>>>>> socfpga/clock_manager.c >>>>>>>>>>>>> b/arch/arm/mach- >>>>>>>>>>>>> socfpga/clock_manager.c >>>>>>>>>>>>> index aa71636..0d67b3c 100644 >>>>>>>>>>>>> --- a/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>>>>>> +++ b/arch/arm/mach-socfpga/clock_manager.c >>>>>>>>>>>>> @@ -10,6 +10,7 @@ >>>>>>>>>>>>> >>>>>>>>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>>>>>>>> >>>>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>>>> static const struct socfpga_clock_manager >>>>>>>>>>>>> *clock_manager_base >>>>>>>>>>>>> = >>>>>>>>>>>>> (struct socfpga_clock_manager >>>>>>>>>>>>> *)SOCFPGA_CLKMGR_ADDRESS; >>>>>>>>>>>>> >>>>>>>>>>>>> @@ -446,9 +447,11 @@ unsigned int >>>>>>>>>>>>> cm_get_l4_sp_clk_hz(void) >>>>>>>>>>>>> >>>>>>>>>>>>> return clock; >>>>>>>>>>>>> } >>>>>>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>>>>>> >>>>>>>>>>>>> unsigned int >>>>>>>>>>>>> cm_get_mmc_controller_clk_hz(void) >>>>>>>>>>>>> { >>>>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>>>>>> >>>>>>>>>>>>> /* identify the source of MMC clock */ >>>>>>>>>>>>> @@ -475,8 +478,12 @@ unsigned int >>>>>>>>>>>>> cm_get_mmc_controller_clk_hz(void) >>>>>>>>>>>>> /* further divide by 4 as we have fixed >>>>>>>>>>>>> divider >>>>>>>>>>>>> at >>>>>>>>>>>>> wrapper */ >>>>>>>>>>>>> clock /= 4; >>>>>>>>>>>>> return clock; >>>>>>>>>>>>> +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) >>>>>>>>>>>>> + return 25000000; >>>>>>>>>>>> Is this always gonna be the case or is this S10VP >>>>>>>>>>>> specific ? >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ >>>>>>>>>>>>> } >>>>>>>>>>>>> >>>>>>>>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) >>>>>>>>>>>>> unsigned int >>>>>>>>>>>>> cm_get_qspi_controller_clk_hz(void) >>>>>>>>>>>>> { >>>>>>>>>>>>> uint32_t reg, clock = 0; >>>>>>>>>>>>> @@ -556,3 +563,4 @@ U_BOOT_CMD( >>>>>>>>>>>>> "display clocks", >>>>>>>>>>>>> "" >>>>>>>>>>>> Why does the clock display not work on S10 ? Are >>>>>>>>>>>> some >>>>>>>>>>>> functions >>>>>>>>>>>> missing? >>>>>>>>>>> Not for SOCVP. But will be added in later stage >>>>>>>>>>> when >>>>>>>>>>> testing >>>>>>>>>>> against >>>>>>>>>>> emulation >>>>>>>>>> How hard would it be to add this missing >>>>>>>>>> functionality >>>>>>>>>> now ? >>>>>>>>>> >>>>>>>>> That will take weeks as that need to be validated as >>>>>>>>> whole >>>>>>>>> in >>>>>>>>> emulation >>>>>>>>> platform. >>>>>>>> You mean printing a few clock information based on some >>>>>>>> values >>>>>>>> from >>>>>>>> registers would take weeks ? Why ? >>>>>>>> >>>>>>> Oh actually I am referring all the managers code such as >>>>>>> full >>>>>>> Clock >>>>>>> Manager, Reset Manager ... plus testing. Testing is the >>>>>>> part >>>>>>> take >>>>>>> some >>>>>>> significant time especially slow when come to emulation. >>>>>> Just use empty functions for the clock init code (since it's >>>>>> not >>>>>> needed >>>>>> on the socvp) and populate the clock reporting functions. >>>>>> That >>>>>> should >>>>>> be >>>>>> simple, right ? >>>>> Can be done but the value won't be meaningful as the register >>>>> is >>>>> uninitialzied. Unless we hardcode to a hard value which might >>>>> not >>>>> sound >>>>> right. >>>> Ha, I see. Is there some sane default for the SoCVP ? >>>> >>> It will not meaningful as performance is same even with frequency >>> change :) But definitely will address this in coming weeks as I am >>> already working on the emulation. >> I understand that, but I believe it is still better to provide some >> sensible value rather than just ifdef the whole code out. > > I can hardcode some sensible value for that if good. I think it's better, yes. Thanks
diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach-socfpga/clock_manager.c index aa71636..0d67b3c 100644 --- a/arch/arm/mach-socfpga/clock_manager.c +++ b/arch/arm/mach-socfpga/clock_manager.c @@ -10,6 +10,7 @@ DECLARE_GLOBAL_DATA_PTR; +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) static const struct socfpga_clock_manager *clock_manager_base = (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; @@ -446,9 +447,11 @@ unsigned int cm_get_l4_sp_clk_hz(void) return clock; } +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ unsigned int cm_get_mmc_controller_clk_hz(void) { +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) uint32_t reg, clock = 0; /* identify the source of MMC clock */ @@ -475,8 +478,12 @@ unsigned int cm_get_mmc_controller_clk_hz(void) /* further divide by 4 as we have fixed divider at wrapper */ clock /= 4; return clock; +#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) + return 25000000; +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */ } +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) unsigned int cm_get_qspi_controller_clk_hz(void) { uint32_t reg, clock = 0; @@ -556,3 +563,4 @@ U_BOOT_CMD( "display clocks", "" ); +#endif /* CONFIG_TARGET_SOCFPGA_GEN5 */
Separate the Clock Manager to support both GEN5 SoC and Stratix 10 SoC. Signed-off-by: Chin Liang See <clsee@altera.com> Cc: Marek Vasut <marex@denx.de> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> Cc: Ley Foon Tan <lftan@altera.com> Cc: Tien Fong Chee <tfchee@altera.com> --- arch/arm/mach-socfpga/clock_manager.c | 8 ++++++++ 1 file changed, 8 insertions(+)