Re: [U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

2016-10-24 Thread york sun
On 10/23/2016 11:29 PM, Priyanka Jain wrote:
>
>
>> -Original Message-
>> From: york sun
>> Sent: Thursday, October 20, 2016 10:46 AM
>> To: Prabhakar Kushwaha ; u-
>> b...@lists.denx.de
>> Cc: Priyanka Jain 
>> Subject: Re: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address
>> detection
>>
>> On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote:
>>> Hi York
>>>
 -Original Message-
 From: Priyanka Jain [mailto:priyanka.j...@nxp.com]
 Sent: Wednesday, October 19, 2016 3:07 PM
 To: u-boot@lists.denx.de
 Cc: Priyanka Jain ; Prabhakar Kushwaha
 
 Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address
 detection

 Timer base address has been changed from LS2080A SoC to new SoCs like
 LS2088A, LS1088A.

 Use SVR based timer base address detection to avoid compile time #ifdef.

 Signed-off-by: Priyanka Jain 
 Signed-off-by: Prabhakar Kushwaha 
 ---
  arch/arm/cpu/armv8/fsl-layerscape/cpu.c|   14 +-
  .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |3 ++-
  2 files changed, 15 insertions(+), 2 deletions(-)

 diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
 b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
 index b7a2e0c..ce04e48 100644
 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
 +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
 @@ -424,8 +424,10 @@ int arch_early_init_r(void)

  int timer_init(void)
  {
 -  u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
 +  u32 __iomem *cntcr;
  #ifdef CONFIG_FSL_LSCH3
 +  struct ccsr_gur __iomem *gur = (void
 *)(CONFIG_SYS_FSL_GUTS_ADDR);
 +  u32 svr, ver;
u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
>> #endif
 #ifdef CONFIG_LS2080A @@ -439,6 +441,16 @@ int timer_init(void)
 #endif

  #ifdef CONFIG_FSL_LSCH3
 +  svr = gur_in32(>svr);
 +  ver = SVR_SOC_VER(svr);
 +  if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) ||
 +  (ver == SVR_LS2085A) || (ver == SVR_LS2045A))
 +  cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR;
 +  else
 +#endif
 +  cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
 +
 +#ifdef CONFIG_FSL_LSCH3
/* Enable timebase for all clusters.
 * It is safe to do so even some clusters are not enabled.
 */
 diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
 b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
 index 7acba27..e6cdfcb 100644
 --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
 +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
 @@ -23,7 +23,8 @@
  #define CONFIG_SYS_IFC_ADDR   (CONFIG_SYS_IMMR
>> +
 0x0124)
  #define CONFIG_SYS_NS16550_COM1
>>  (CONFIG_SYS_IMMR +
 0x011C0500)
  #define CONFIG_SYS_NS16550_COM2
>>  (CONFIG_SYS_IMMR +
 0x011C0600)
 -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d
 +#define LS2080A_LS2085A_TIMER_ADDR0x023d
>>>
>>> Is this ok to define new constant without CONFIG_?
>>> Only problem, it is not consistent with existing defines.
>>>
>>
>> Prabhakar,
>>
>> I opinion on this is
>>
>> 1) Let's check if a Kconfig option is better for it If yes, let's convert it 
>> to
>> Kconfig. If not, go to 2)
>>
>> 2) Let's use a different name space for the macros.
>> I suggest to use SYS_FSL_* for fixed value for our hardware.
>>
>> York
>
> LS2080A_LS2085A_TIMER_ADDR is defining the timer controller (register) offset.
> Kconfig option does not look to be very helpful here.
> I am thinking of renaming this to "SYS_FSL_LS2080A_LS2085A_TIMER_ADDR"
>
> Please confirm if this is fine.
>

Sorry for late reply. The new name is OK. Please consider to change 
other existing CONFIG_* macros to SYS_FSL_* space, and remove them from 
the white list.

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


Re: [U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

2016-10-24 Thread Priyanka Jain


> -Original Message-
> From: york sun
> Sent: Thursday, October 20, 2016 10:46 AM
> To: Prabhakar Kushwaha ; u-
> b...@lists.denx.de
> Cc: Priyanka Jain 
> Subject: Re: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address
> detection
> 
> On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote:
> > Hi York
> >
> >> -Original Message-
> >> From: Priyanka Jain [mailto:priyanka.j...@nxp.com]
> >> Sent: Wednesday, October 19, 2016 3:07 PM
> >> To: u-boot@lists.denx.de
> >> Cc: Priyanka Jain ; Prabhakar Kushwaha
> >> 
> >> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address
> >> detection
> >>
> >> Timer base address has been changed from LS2080A SoC to new SoCs like
> >> LS2088A, LS1088A.
> >>
> >> Use SVR based timer base address detection to avoid compile time #ifdef.
> >>
> >> Signed-off-by: Priyanka Jain 
> >> Signed-off-by: Prabhakar Kushwaha 
> >> ---
> >>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c|   14 +-
> >>  .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |3 ++-
> >>  2 files changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >> index b7a2e0c..ce04e48 100644
> >> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >> @@ -424,8 +424,10 @@ int arch_early_init_r(void)
> >>
> >>  int timer_init(void)
> >>  {
> >> -  u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
> >> +  u32 __iomem *cntcr;
> >>  #ifdef CONFIG_FSL_LSCH3
> >> +  struct ccsr_gur __iomem *gur = (void
> >> *)(CONFIG_SYS_FSL_GUTS_ADDR);
> >> +  u32 svr, ver;
> >>u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
> #endif
> >> #ifdef CONFIG_LS2080A @@ -439,6 +441,16 @@ int timer_init(void)
> >> #endif
> >>
> >>  #ifdef CONFIG_FSL_LSCH3
> >> +  svr = gur_in32(>svr);
> >> +  ver = SVR_SOC_VER(svr);
> >> +  if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) ||
> >> +  (ver == SVR_LS2085A) || (ver == SVR_LS2045A))
> >> +  cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR;
> >> +  else
> >> +#endif
> >> +  cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
> >> +
> >> +#ifdef CONFIG_FSL_LSCH3
> >>/* Enable timebase for all clusters.
> >> * It is safe to do so even some clusters are not enabled.
> >> */
> >> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >> index 7acba27..e6cdfcb 100644
> >> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> >> @@ -23,7 +23,8 @@
> >>  #define CONFIG_SYS_IFC_ADDR   (CONFIG_SYS_IMMR
> +
> >> 0x0124)
> >>  #define CONFIG_SYS_NS16550_COM1
>   (CONFIG_SYS_IMMR +
> >> 0x011C0500)
> >>  #define CONFIG_SYS_NS16550_COM2
>   (CONFIG_SYS_IMMR +
> >> 0x011C0600)
> >> -#define CONFIG_SYS_FSL_TIMER_ADDR 0x023d
> >> +#define LS2080A_LS2085A_TIMER_ADDR0x023d
> >
> > Is this ok to define new constant without CONFIG_?
> > Only problem, it is not consistent with existing defines.
> >
> 
> Prabhakar,
> 
> I opinion on this is
> 
> 1) Let's check if a Kconfig option is better for it If yes, let's convert it 
> to
> Kconfig. If not, go to 2)
> 
> 2) Let's use a different name space for the macros.
> I suggest to use SYS_FSL_* for fixed value for our hardware.
> 
> York

LS2080A_LS2085A_TIMER_ADDR is defining the timer controller (register) offset.
Kconfig option does not look to be very helpful here.
I am thinking of renaming this to "SYS_FSL_LS2080A_LS2085A_TIMER_ADDR"

Please confirm if this is fine.

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


Re: [U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

2016-10-19 Thread york sun
On 10/19/2016 10:28 PM, Prabhakar Kushwaha wrote:
> Hi York
>
>> -Original Message-
>> From: Priyanka Jain [mailto:priyanka.j...@nxp.com]
>> Sent: Wednesday, October 19, 2016 3:07 PM
>> To: u-boot@lists.denx.de
>> Cc: Priyanka Jain ; Prabhakar Kushwaha
>> 
>> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection
>>
>> Timer base address has been changed from LS2080A SoC to
>> new SoCs like LS2088A, LS1088A.
>>
>> Use SVR based timer base address detection to avoid compile time #ifdef.
>>
>> Signed-off-by: Priyanka Jain 
>> Signed-off-by: Prabhakar Kushwaha 
>> ---
>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c|   14 +-
>>  .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |3 ++-
>>  2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> index b7a2e0c..ce04e48 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> @@ -424,8 +424,10 @@ int arch_early_init_r(void)
>>
>>  int timer_init(void)
>>  {
>> -u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
>> +u32 __iomem *cntcr;
>>  #ifdef CONFIG_FSL_LSCH3
>> +struct ccsr_gur __iomem *gur = (void
>> *)(CONFIG_SYS_FSL_GUTS_ADDR);
>> +u32 svr, ver;
>>  u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
>>  #endif
>>  #ifdef CONFIG_LS2080A
>> @@ -439,6 +441,16 @@ int timer_init(void)
>>  #endif
>>
>>  #ifdef CONFIG_FSL_LSCH3
>> +svr = gur_in32(>svr);
>> +ver = SVR_SOC_VER(svr);
>> +if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) ||
>> +(ver == SVR_LS2085A) || (ver == SVR_LS2045A))
>> +cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR;
>> +else
>> +#endif
>> +cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
>> +
>> +#ifdef CONFIG_FSL_LSCH3
>>  /* Enable timebase for all clusters.
>>   * It is safe to do so even some clusters are not enabled.
>>   */
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> index 7acba27..e6cdfcb 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
>> @@ -23,7 +23,8 @@
>>  #define CONFIG_SYS_IFC_ADDR (CONFIG_SYS_IMMR +
>> 0x0124)
>>  #define CONFIG_SYS_NS16550_COM1 (CONFIG_SYS_IMMR +
>> 0x011C0500)
>>  #define CONFIG_SYS_NS16550_COM2 (CONFIG_SYS_IMMR +
>> 0x011C0600)
>> -#define CONFIG_SYS_FSL_TIMER_ADDR   0x023d
>> +#define LS2080A_LS2085A_TIMER_ADDR  0x023d
>
> Is this ok to define new constant without CONFIG_?
> Only problem, it is not consistent with existing defines.
>

Prabhakar,

I opinion on this is

1) Let's check if a Kconfig option is better for it
If yes, let's convert it to Kconfig. If not, go to 2)

2) Let's use a different name space for the macros.
I suggest to use SYS_FSL_* for fixed value for our hardware.

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


Re: [U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

2016-10-19 Thread Prabhakar Kushwaha
Hi York

> -Original Message-
> From: Priyanka Jain [mailto:priyanka.j...@nxp.com]
> Sent: Wednesday, October 19, 2016 3:07 PM
> To: u-boot@lists.denx.de
> Cc: Priyanka Jain ; Prabhakar Kushwaha
> 
> Subject: [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection
> 
> Timer base address has been changed from LS2080A SoC to
> new SoCs like LS2088A, LS1088A.
> 
> Use SVR based timer base address detection to avoid compile time #ifdef.
> 
> Signed-off-by: Priyanka Jain 
> Signed-off-by: Prabhakar Kushwaha 
> ---
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c|   14 +-
>  .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |3 ++-
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index b7a2e0c..ce04e48 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -424,8 +424,10 @@ int arch_early_init_r(void)
> 
>  int timer_init(void)
>  {
> - u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
> + u32 __iomem *cntcr;
>  #ifdef CONFIG_FSL_LSCH3
> + struct ccsr_gur __iomem *gur = (void
> *)(CONFIG_SYS_FSL_GUTS_ADDR);
> + u32 svr, ver;
>   u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
>  #endif
>  #ifdef CONFIG_LS2080A
> @@ -439,6 +441,16 @@ int timer_init(void)
>  #endif
> 
>  #ifdef CONFIG_FSL_LSCH3
> + svr = gur_in32(>svr);
> + ver = SVR_SOC_VER(svr);
> + if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) ||
> + (ver == SVR_LS2085A) || (ver == SVR_LS2045A))
> + cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR;
> + else
> +#endif
> + cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
> +
> +#ifdef CONFIG_FSL_LSCH3
>   /* Enable timebase for all clusters.
>* It is safe to do so even some clusters are not enabled.
>*/
> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> index 7acba27..e6cdfcb 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
> @@ -23,7 +23,8 @@
>  #define CONFIG_SYS_IFC_ADDR  (CONFIG_SYS_IMMR +
> 0x0124)
>  #define CONFIG_SYS_NS16550_COM1  (CONFIG_SYS_IMMR +
> 0x011C0500)
>  #define CONFIG_SYS_NS16550_COM2  (CONFIG_SYS_IMMR +
> 0x011C0600)
> -#define CONFIG_SYS_FSL_TIMER_ADDR0x023d
> +#define LS2080A_LS2085A_TIMER_ADDR   0x023d

Is this ok to define new constant without CONFIG_?
Only problem, it is not consistent with existing defines. 

-prabhakar

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


[U-Boot] [PATCH 1/5] armv8: lsch3: Use SVR based timer base address detection

2016-10-19 Thread Priyanka Jain
Timer base address has been changed from LS2080A SoC to
new SoCs like LS2088A, LS1088A.

Use SVR based timer base address detection to avoid compile time #ifdef.

Signed-off-by: Priyanka Jain 
Signed-off-by: Prabhakar Kushwaha 
---
 arch/arm/cpu/armv8/fsl-layerscape/cpu.c|   14 +-
 .../include/asm/arch-fsl-layerscape/immap_lsch3.h  |3 ++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c 
b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index b7a2e0c..ce04e48 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -424,8 +424,10 @@ int arch_early_init_r(void)
 
 int timer_init(void)
 {
-   u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
+   u32 __iomem *cntcr;
 #ifdef CONFIG_FSL_LSCH3
+   struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+   u32 svr, ver;
u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
 #endif
 #ifdef CONFIG_LS2080A
@@ -439,6 +441,16 @@ int timer_init(void)
 #endif
 
 #ifdef CONFIG_FSL_LSCH3
+   svr = gur_in32(>svr);
+   ver = SVR_SOC_VER(svr);
+   if ((ver == SVR_LS2080A) || (ver == SVR_LS2040A) ||
+   (ver == SVR_LS2085A) || (ver == SVR_LS2045A))
+   cntcr = (u32 *)LS2080A_LS2085A_TIMER_ADDR;
+   else
+#endif
+   cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
+
+#ifdef CONFIG_FSL_LSCH3
/* Enable timebase for all clusters.
 * It is safe to do so even some clusters are not enabled.
 */
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h 
b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 7acba27..e6cdfcb 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -23,7 +23,8 @@
 #define CONFIG_SYS_IFC_ADDR(CONFIG_SYS_IMMR + 0x0124)
 #define CONFIG_SYS_NS16550_COM1(CONFIG_SYS_IMMR + 
0x011C0500)
 #define CONFIG_SYS_NS16550_COM2(CONFIG_SYS_IMMR + 
0x011C0600)
-#define CONFIG_SYS_FSL_TIMER_ADDR  0x023d
+#define LS2080A_LS2085A_TIMER_ADDR 0x023d
+#define CONFIG_SYS_FSL_TIMER_ADDR  0x023e
 #define CONFIG_SYS_FSL_PMU_CLTBENR (CONFIG_SYS_FSL_PMU_ADDR + \
 0x18A0)
 #define FSL_PMU_PCTBENR_OFFSET (CONFIG_SYS_FSL_PMU_ADDR + 0x8A0)
-- 
1.7.4.1

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