Re: [PATCH v2 08/21] sunxi: introduce support for H616 clocks

2021-01-21 Thread Jernej Škrabec
Dne petek, 22. januar 2021 ob 02:17:30 CET je Andre Przywara napisal(a):
> On Mon, 11 Jan 2021 21:11:40 +0100
> 
> Jernej Skrabec  wrote:
> > H616 has mostly the same clocks as H6 with some small differences. Just
> > reuse H6 clocks for H616 and handle differences with macros.
> > 
> > Reviewed-by: Samuel Holland 
> > Signed-off-by: Jernej Skrabec 
> > ---
> > 
> >  .../include/asm/arch-sunxi/clock_sun50i_h6.h   | 18 +-
> >  arch/arm/mach-sunxi/clock_sun50i_h6.c  |  8 ++--
> >  2 files changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> > b/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h index
> > e83e84ab6cab..62abfc4ef6bd 100644
> > --- a/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> > +++ b/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> > @@ -230,6 +230,7 @@ struct sunxi_ccm_reg {
> > 
> >  #define CCM_PLL1_CTRL_EN   BIT(31)
> >  #define CCM_PLL1_LOCK_EN   BIT(29)
> >  #define CCM_PLL1_LOCK  BIT(28)
> > 
> > +#define CCM_PLL1_OUT_ENBIT(27)
> > 
> >  #define CCM_PLL1_CLOCK_TIME_2  (2 << 24)
> >  #define CCM_PLL1_CTRL_P(p) ((p) << 16)
> >  #define CCM_PLL1_CTRL_N(n) ((n) << 8)
> > 
> > @@ -238,6 +239,7 @@ struct sunxi_ccm_reg {
> > 
> >  #define CCM_PLL5_CTRL_EN   BIT(31)
> >  #define CCM_PLL5_LOCK_EN   BIT(29)
> >  #define CCM_PLL5_LOCK  BIT(28)
> > 
> > +#define CCM_PLL5_OUT_ENBIT(27)
> > 
> >  #define CCM_PLL5_CTRL_N(n) ((n) << 8)
> >  #define CCM_PLL5_CTRL_DIV1(div1)   ((div1) << 0)
> >  #define CCM_PLL5_CTRL_DIV2(div0)   ((div0) << 1)
> > 
> > @@ -252,7 +254,6 @@ struct sunxi_ccm_reg {
> > 
> >  #define CCM_PLL6_CTRL_DIV1_MASK(0x1 << 
CCM_PLL6_CTRL_DIV1_SHIFT)
> >  #define CCM_PLL6_CTRL_DIV2_SHIFT   1
> >  #define CCM_PLL6_CTRL_DIV2_MASK(0x1 << 
CCM_PLL6_CTRL_DIV2_SHIFT)
> > 
> > -#define CCM_PLL6_DEFAULT   0xa0006300
> > 
> >  /* cpu_axi bit field*/
> >  #define CCM_CPU_AXI_MUX_MASK   (0x3 << 24)
> > 
> > @@ -262,6 +263,9 @@ struct sunxi_ccm_reg {
> > 
> >  #define CCM_CPU_AXI_AXI_MASK   0x3
> >  #define CCM_CPU_AXI_DEFAULT_FACTORS0x301
> > 
> > +#ifdef CONFIG_MACH_SUN50I_H6
> > +#define CCM_PLL6_DEFAULT   0xa0006300
> > +
> > 
> >  /* psi_ahb1_ahb2 bit field */
> >  #define CCM_PSI_AHB1_AHB2_DEFAULT  0x03000102
> > 
> > @@ -270,6 +274,18 @@ struct sunxi_ccm_reg {
> > 
> >  /* apb1 bit field */
> >  #define CCM_APB1_DEFAULT   0x03000102
> > 
> > +#elif CONFIG_MACH_SUN50I_H616
> > +#define CCM_PLL6_DEFAULT   0xa8003100
> > +
> > +/* psi_ahb1_ahb2 bit field */
> > +#define CCM_PSI_AHB1_AHB2_DEFAULT  0x0302
> 
> Why is this twice as fast, compared to the H6? It's based on the same
> PERI0(1X), which should be 600 MHz for both the H6 and H616, right?
> 
> Is that the reset config or set up by the BROM? Or is that coming from
> some BSP source?

Actually I just dumped register when Android was running.

Best regards,
Jernej

> 
> > +
> > +/* ahb3 bit field */
> > +#define CCM_AHB3_DEFAULT   0x0302
> > +
> > +/* apb1 bit field */
> > +#define CCM_APB1_DEFAULT   0x03000102
> > +#endif
> > 
> >  /* apb2 bit field */
> >  #define APB2_CLK_SRC_OSC24M(0x0 << 24)
> > 
> > diff --git a/arch/arm/mach-sunxi/clock_sun50i_h6.c
> > b/arch/arm/mach-sunxi/clock_sun50i_h6.c index 6bd466915c11..daca02019bab
> > 100644
> > --- a/arch/arm/mach-sunxi/clock_sun50i_h6.c
> > +++ b/arch/arm/mach-sunxi/clock_sun50i_h6.c
> > @@ -68,6 +68,9 @@ void clock_set_pll1(unsigned int clk)
> > 
> > /* clk = 24*n/p, p is ignored if clock is >288MHz */
> > writel(CCM_PLL1_CTRL_EN | CCM_PLL1_LOCK_EN | CCM_PLL1_CLOCK_TIME_2 
|
> > 
> > +#ifdef CONFIG_MACH_SUN50I_H616
> > +  CCM_PLL1_OUT_EN |
> > +#endif
> > 
> >CCM_PLL1_CTRL_N(clk / 2400), &ccm->pll1_cfg);
> > 
> > while (!(readl(&ccm->pll1_cfg) & CCM_PLL1_LOCK)) {}
> > 
> > @@ -83,6 +86,7 @@ unsigned int clock_get_pll6(void)
> > 
> >  {
> >  
> > struct sunxi_ccm_reg *const ccm =
> > 
> > (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> > 
> > +   int m = IS_ENABLED(CONFIG_MACH_SUN50I_H6) 4 : 2;
> 
> This is missing the question mark, but that gets fixed up in a later
> patch, so I assume it's a rebase artifact. Will fix it up while
> committing.
> 
> Cheers,
> Andre
> 
> > uint32_t rval = readl(&ccm->pll6_cfg);
> > int n = ((rval & CCM_PLL6_CTRL_N_MASK) >> CCM_PLL6_CTRL_N_SHIFT);
> > 
> > @@ -90,8 +94,8 @@ unsigned int clock_get_pll6(void)
> > 
> > CCM_PLL6_CTRL_DIV1_SHIFT) + 1;
> > 
> > int div2 = ((rval & CCM_PLL6_CTRL_DIV2_MASK) >>
> > 
> > CCM_PLL6_CTRL_DIV2_SHIFT) + 1;
> > 
> > -   /* The register defines PLL6-4X, not plain PLL6 */
> > -   return 2400 / 4 * n / div1 / div2;
> > +   /* The register

Re: [PATCH v2 08/21] sunxi: introduce support for H616 clocks

2021-01-21 Thread Andre Przywara
On Mon, 11 Jan 2021 21:11:40 +0100
Jernej Skrabec  wrote:

> H616 has mostly the same clocks as H6 with some small differences. Just
> reuse H6 clocks for H616 and handle differences with macros.
> 
> Reviewed-by: Samuel Holland 
> Signed-off-by: Jernej Skrabec 
> ---
>  .../include/asm/arch-sunxi/clock_sun50i_h6.h   | 18 +-
>  arch/arm/mach-sunxi/clock_sun50i_h6.c  |  8 ++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h 
> b/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> index e83e84ab6cab..62abfc4ef6bd 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun50i_h6.h
> @@ -230,6 +230,7 @@ struct sunxi_ccm_reg {
>  #define CCM_PLL1_CTRL_EN BIT(31)
>  #define CCM_PLL1_LOCK_EN BIT(29)
>  #define CCM_PLL1_LOCKBIT(28)
> +#define CCM_PLL1_OUT_EN  BIT(27)
>  #define CCM_PLL1_CLOCK_TIME_2(2 << 24)
>  #define CCM_PLL1_CTRL_P(p)   ((p) << 16)
>  #define CCM_PLL1_CTRL_N(n)   ((n) << 8)
> @@ -238,6 +239,7 @@ struct sunxi_ccm_reg {
>  #define CCM_PLL5_CTRL_EN BIT(31)
>  #define CCM_PLL5_LOCK_EN BIT(29)
>  #define CCM_PLL5_LOCKBIT(28)
> +#define CCM_PLL5_OUT_EN  BIT(27)
>  #define CCM_PLL5_CTRL_N(n)   ((n) << 8)
>  #define CCM_PLL5_CTRL_DIV1(div1) ((div1) << 0)
>  #define CCM_PLL5_CTRL_DIV2(div0) ((div0) << 1)
> @@ -252,7 +254,6 @@ struct sunxi_ccm_reg {
>  #define CCM_PLL6_CTRL_DIV1_MASK  (0x1 << 
> CCM_PLL6_CTRL_DIV1_SHIFT)
>  #define CCM_PLL6_CTRL_DIV2_SHIFT 1
>  #define CCM_PLL6_CTRL_DIV2_MASK  (0x1 << 
> CCM_PLL6_CTRL_DIV2_SHIFT)
> -#define CCM_PLL6_DEFAULT 0xa0006300
>  
>  /* cpu_axi bit field*/
>  #define CCM_CPU_AXI_MUX_MASK (0x3 << 24)
> @@ -262,6 +263,9 @@ struct sunxi_ccm_reg {
>  #define CCM_CPU_AXI_AXI_MASK 0x3
>  #define CCM_CPU_AXI_DEFAULT_FACTORS  0x301
>  
> +#ifdef CONFIG_MACH_SUN50I_H6
> +#define CCM_PLL6_DEFAULT 0xa0006300
> +
>  /* psi_ahb1_ahb2 bit field */
>  #define CCM_PSI_AHB1_AHB2_DEFAULT0x03000102
>  
> @@ -270,6 +274,18 @@ struct sunxi_ccm_reg {
>  
>  /* apb1 bit field */
>  #define CCM_APB1_DEFAULT 0x03000102
> +#elif CONFIG_MACH_SUN50I_H616
> +#define CCM_PLL6_DEFAULT 0xa8003100
> +
> +/* psi_ahb1_ahb2 bit field */
> +#define CCM_PSI_AHB1_AHB2_DEFAULT0x0302

Why is this twice as fast, compared to the H6? It's based on the same
PERI0(1X), which should be 600 MHz for both the H6 and H616, right?

Is that the reset config or set up by the BROM? Or is that coming from
some BSP source?

> +
> +/* ahb3 bit field */
> +#define CCM_AHB3_DEFAULT 0x0302
> +
> +/* apb1 bit field */
> +#define CCM_APB1_DEFAULT 0x03000102
> +#endif
>  
>  /* apb2 bit field */
>  #define APB2_CLK_SRC_OSC24M  (0x0 << 24)
> diff --git a/arch/arm/mach-sunxi/clock_sun50i_h6.c 
> b/arch/arm/mach-sunxi/clock_sun50i_h6.c
> index 6bd466915c11..daca02019bab 100644
> --- a/arch/arm/mach-sunxi/clock_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/clock_sun50i_h6.c
> @@ -68,6 +68,9 @@ void clock_set_pll1(unsigned int clk)
>  
>   /* clk = 24*n/p, p is ignored if clock is >288MHz */
>   writel(CCM_PLL1_CTRL_EN | CCM_PLL1_LOCK_EN | CCM_PLL1_CLOCK_TIME_2 |
> +#ifdef CONFIG_MACH_SUN50I_H616
> +CCM_PLL1_OUT_EN |
> +#endif
>  CCM_PLL1_CTRL_N(clk / 2400), &ccm->pll1_cfg);
>   while (!(readl(&ccm->pll1_cfg) & CCM_PLL1_LOCK)) {}
>  
> @@ -83,6 +86,7 @@ unsigned int clock_get_pll6(void)
>  {
>   struct sunxi_ccm_reg *const ccm =
>   (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> + int m = IS_ENABLED(CONFIG_MACH_SUN50I_H6) 4 : 2;

This is missing the question mark, but that gets fixed up in a later
patch, so I assume it's a rebase artifact. Will fix it up while
committing.

Cheers,
Andre

>  
>   uint32_t rval = readl(&ccm->pll6_cfg);
>   int n = ((rval & CCM_PLL6_CTRL_N_MASK) >> CCM_PLL6_CTRL_N_SHIFT);
> @@ -90,8 +94,8 @@ unsigned int clock_get_pll6(void)
>   CCM_PLL6_CTRL_DIV1_SHIFT) + 1;
>   int div2 = ((rval & CCM_PLL6_CTRL_DIV2_MASK) >>
>   CCM_PLL6_CTRL_DIV2_SHIFT) + 1;
> - /* The register defines PLL6-4X, not plain PLL6 */
> - return 2400 / 4 * n / div1 / div2;
> + /* The register defines PLL6-2X or PLL6-4X, not plain PLL6 */
> + return 2400 / m * n / div1 / div2;
>  }
>  
>  int clock_twi_onoff(int port, int state)