Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 >
> Cc: Marek Vasut 
> Cc: Dinh Nguyen >
> Cc: Ley Foon Tan 
> Cc: Tien Fong Chee 
> ---
>  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 2500;
 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 

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

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  > > > > > > > > > > > >
> > > > > > > > > > > > Cc: Marek Vasut 
> > > > > > > > > > > > Cc: Dinh Nguyen  > > > > > > > > > > > >
> > > > > > > > > > > > Cc: Ley Foon Tan 
> > > > > > > > > > > > Cc: Tien Fong Chee 
> > > > > > > > > > > > ---
> > > > > > > > > > > >  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 2500;
> > > > > > > > > > > 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 

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

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 
> > > > > > > > > > Cc: Marek Vasut 
> > > > > > > > > > Cc: Dinh Nguyen 
> > > > > > > > > > Cc: Ley Foon Tan 
> > > > > > > > > > Cc: Tien Fong Chee 
> > > > > > > > > > ---
> > > > > > > > > >  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 2500;
> > > > > > > > > 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 

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 
>>> Cc: Marek Vasut 
>>> Cc: Dinh Nguyen 
>>> Cc: Ley Foon Tan 
>>> Cc: Tien Fong Chee 
>>> ---
>>>  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 2500;
>> 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.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[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 
> Cc: Marek Vasut 
> Cc: Dinh Nguyen 
> Cc: Ley Foon Tan 
> Cc: Tien Fong Chee 
> ---
>  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 2500;
 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 ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

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 
> > > > > > > > Cc: Marek Vasut 
> > > > > > > > Cc: Dinh Nguyen 
> > > > > > > > Cc: Ley Foon Tan 
> > > > > > > > Cc: Tien Fong Chee 
> > > > > > > > ---
> > > > > > > >  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 2500;
> > > > > > > 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
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 
>>> Cc: Marek Vasut 
>>> Cc: Dinh Nguyen 
>>> Cc: Ley Foon Tan 
>>> Cc: Tien Fong Chee 
>>> ---
>>>  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 2500;
>> 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 ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

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 
> > > > > > Cc: Marek Vasut 
> > > > > > Cc: Dinh Nguyen 
> > > > > > Cc: Ley Foon Tan 
> > > > > > Cc: Tien Fong Chee 
> > > > > > ---
> > > > > >  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 2500;
> > > > > 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

[..]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 
> Cc: Marek Vasut 
> Cc: Dinh Nguyen 
> Cc: Ley Foon Tan 
> Cc: Tien Fong Chee 
> ---
>  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 2500;
 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
> 
> 
>> [...]
>>
>>


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Chin Liang See]

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 
> > > > Cc: Marek Vasut 
> > > > Cc: Dinh Nguyen 
> > > > Cc: Ley Foon Tan 
> > > > Cc: Tien Fong Chee 
> > > > ---
> > > >  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 2500;
> > > 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


> [...]
> 
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 
>>> Cc: Marek Vasut 
>>> Cc: Dinh Nguyen 
>>> Cc: Ley Foon Tan 
>>> Cc: Tien Fong Chee 
>>> ---
>>>  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 2500;
>> 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.

[...]


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[See, Chin Liang]

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 
> > Cc: Marek Vasut 
> > Cc: Dinh Nguyen 
> > Cc: Ley Foon Tan 
> > Cc: Tien Fong Chee 
> > ---
> >  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 2500;
> 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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 04/12] arm: socfpga: clkmgr: Separate the Clock Manager for Stratix 10

[Marek Vasut]

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 
> Cc: Marek Vasut 
> Cc: Dinh Nguyen 
> Cc: Ley Foon Tan 
> Cc: Tien Fong Chee 
> ---
>  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 2500;

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 */
> 


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot