On Tue, 25 Jan 2022 23:53:27 -0500
Jesse Taube <mr.bossman...@gmail.com> wrote:

Hi Jesse,

> On 1/25/22 21:05, Andre Przywara wrote:
> > On Tue,  4 Jan 2022 19:35:02 -0500
> > Jesse Taube <mr.bossman...@gmail.com> wrote:
> > 
> > Hi,
> > 
> > can you please change the subject, to maybe mention clocks? Reads
> > rather generic as is.
> >   
> >> From: Icenowy Zheng <icen...@aosc.io>
> >>
> >> This patch aims to add header files for the F1C100s.
> >> The header files included add support for gpio, dram and clocks.  
> > 
> > This looks to be about clocks and UARTs only?
> >   
> >>
> >> Signed-off-by: Icenowy Zheng <icen...@aosc.io>
> >> Signed-off-by: Jesse Taube <mr.bossman...@gmail.com>
> >> ---
> >>   arch/arm/include/asm/arch-sunxi/clock.h       |  2 +-
> >>   arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 25 +++++++++++++++++++
> >>   arch/arm/include/asm/arch-sunxi/cpu_sun4i.h   |  8 ++++++
> >>   arch/arm/include/asm/arch-sunxi/gpio.h        |  1 +
> >>   4 files changed, 35 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/include/asm/arch-sunxi/clock.h 
> >> b/arch/arm/include/asm/arch-sunxi/clock.h
> >> index cbbe5c7a1e..2cfd540742 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/clock.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/clock.h
> >> @@ -19,7 +19,7 @@
> >>   #elif defined(CONFIG_SUN50I_GEN_H6)
> >>   #include <asm/arch/clock_sun50i_h6.h>
> >>   #elif defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN8I) || \
> >> -      defined(CONFIG_MACH_SUN50I)
> >> +      defined(CONFIG_MACH_SUN50I) || defined(CONFIG_MACH_SUNIV)
> >>   #include <asm/arch/clock_sun6i.h>
> >>   #elif defined(CONFIG_MACH_SUN9I)
> >>   #include <asm/arch/clock_sun9i.h>
> >> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h 
> >> b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> >> index ee387127f3..5ecdf58bd5 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
> >> @@ -168,6 +168,14 @@ struct sunxi_ccm_reg {
> >>    u32 pll_lock_ctrl;      /* 0x320 PLL lock control, R40 only */
> >>   };
> >>   
> >> +/* apb1 bit field */
> >> +#ifdef CONFIG_MACH_SUNIV
> >> +#define APB1_GATE_UART_SHIFT      (20)
> >> +#define APB1_GATE_UART_MASK               (0x7 << APB1_GATE_UART_SHIFT)
> >> +#define APB1_GATE_TWI_SHIFT       (16)
> >> +#define APB1_GATE_TWI_MASK                (0x7 << APB1_GATE_TWI_SHIFT)  
> > 
> > The values match the user manual, but I don't see the *_MASK values used
> > anywhere. Please just drop them.
> >   
> >> +#endif
> >> +
> >>   /* apb2 bit field */
> >>   #define APB2_CLK_SRC_LOSC                (0x0 << 24)
> >>   #define APB2_CLK_SRC_OSC24M              (0x1 << 24)
> >> @@ -226,7 +234,12 @@ struct sunxi_ccm_reg {
> >>   #define CCM_PLL5_CTRL_SIGMA_DELTA_EN     (0x1 << 24)
> >>   #define CCM_PLL5_CTRL_EN         (0x1 << 31)
> >>   
> >> +#if !defined(CONFIG_MACH_SUNIV)
> >>   #define PLL6_CFG_DEFAULT         0x90041811 /* 600 MHz */
> >> +#else
> >> +/* suniv pll6 doesn't have postdiv 2, so k is set to 0 */
> >> +#define PLL6_CFG_DEFAULT          0x90041800  
> > 
> > The manual says that bit 0 resets to 1, as in the other SoCs. It just
> > seems to affect the "back door clock output", so we should leave it as
> > 1.  
> 
> Uh I don't understand.

If you look at the description of the PLL_PERIPH register (@0x28) in the
CCU section of the F1C200s user manual, you will find that bits 1:0 are
defaulting to 0x1, the same as for the other SoCs handled in this file.
The description says that this affects the "PLL_PERIPH back door clock
output" and is for debug only, which tells me that we should leave it
alone, as we do on the other SoCs. So the value should read 0x90041801.

Cheers,
Andre

> 
> >> +#endif
> >>   
> >>   #define CCM_PLL6_CTRL_N_SHIFT            8
> >>   #define CCM_PLL6_CTRL_N_MASK             (0x1f << CCM_PLL6_CTRL_N_SHIFT)
> >> @@ -310,6 +323,8 @@ struct sunxi_ccm_reg {
> >>   #define AHB_GATE_OFFSET_USB0             25
> >>   #define AHB_GATE_OFFSET_SATA             24
> >>   #endif
> >> +#define AHB_GATE_OFFSET_SPI1              21
> >> +#define AHB_GATE_OFFSET_SPI0              20  
> > 
> > I think this is legacy, we don't need this anymore with the DM clock
> > driver we use in the SPI driver. The SPL SPI is separate anyway.
> > Please just drop.
> >   
> >>   #define AHB_GATE_OFFSET_MCTL             14
> >>   #define AHB_GATE_OFFSET_GMAC             17
> >>   #define AHB_GATE_OFFSET_NAND0            13
> >> @@ -458,6 +473,8 @@ struct sunxi_ccm_reg {
> >>   #ifdef CONFIG_MACH_SUN8I_R40
> >>   #define AHB_RESET_OFFSET_SATA            24
> >>   #endif
> >> +#define AHB_RESET_OFFSET_SPI1             21
> >> +#define AHB_RESET_OFFSET_SPI0             20  
> > 
> > Same here, I don't see that used anywhere.
> >   
> >>   #define AHB_RESET_OFFSET_GMAC            17
> >>   #define AHB_RESET_OFFSET_MCTL            14
> >>   #define AHB_RESET_OFFSET_MMC3            11
> >> @@ -488,6 +505,14 @@ struct sunxi_ccm_reg {
> >>   #define AHB_RESET_OFFSET_EPHY            2
> >>   #define AHB_RESET_OFFSET_LVDS            0
> >>   
> >> +/* apb1 reset */
> >> +#ifdef CONFIG_MACH_SUNIV
> >> +#define APB1_RESET_UART_SHIFT     (20)
> >> +#define APB1_RESET_UART_MASK              (0x7 << APB1_RESET_UART_SHIFT)
> >> +#define APB1_RESET_TWI_SHIFT      (16)
> >> +#define APB1_RESET_TWI_MASK               (0x7 << APB1_RESET_TWI_SHIFT)
> >> +#endif
> >> +
> >>   /* apb2 reset */
> >>   #define APB2_RESET_UART_SHIFT            (16)
> >>   #define APB2_RESET_UART_MASK             (0xff << APB2_RESET_UART_SHIFT)
> >> diff --git a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h 
> >> b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >> index d4c795d89c..83178dd5c8 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/cpu_sun4i.h
> >> @@ -122,6 +122,12 @@ defined(CONFIG_MACH_SUN50I)
> >>   
> >>   #define SUNXI_SJTAG_BASE         0x01c23c00
> >>   
> >> +#ifdef CONFIG_MACH_SUNIV
> >> +#define SUNXI_UART0_BASE          0x01c25000
> >> +#define SUNXI_UART1_BASE          0x01c25400
> >> +#define SUNXI_UART2_BASE          0x01c25800
> >> +#endif  
> > 
> > That should be moved down and merged into the other chunk below.
> > 
> > Rest looks alright.
> > 
> > Cheers,
> > Andre
> > 
> >   
> >> +
> >>   #define SUNXI_TP_BASE                    0x01c25000
> >>   #define SUNXI_PMU_BASE                   0x01c25400
> >>   
> >> @@ -129,9 +135,11 @@ defined(CONFIG_MACH_SUN50I)
> >>   #define SUNXI_CPUCFG_BASE                0x01c25c00
> >>   #endif
> >>   
> >> +#ifndef CONFIG_MACH_SUNIV
> >>   #define SUNXI_UART0_BASE         0x01c28000
> >>   #define SUNXI_UART1_BASE         0x01c28400
> >>   #define SUNXI_UART2_BASE         0x01c28800
> >> +#endif
> >>   #define SUNXI_UART3_BASE         0x01c28c00
> >>   #define SUNXI_UART4_BASE         0x01c29000
> >>   #define SUNXI_UART5_BASE         0x01c29400
> >> diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h 
> >> b/arch/arm/include/asm/arch-sunxi/gpio.h
> >> index f3ab1aea0e..ced69f7dd4 100644
> >> --- a/arch/arm/include/asm/arch-sunxi/gpio.h
> >> +++ b/arch/arm/include/asm/arch-sunxi/gpio.h
> >> @@ -165,6 +165,7 @@ enum sunxi_gpio_number {
> >>   #define SUNXI_GPD_LVDS0          3
> >>   #define SUNXI_GPD_PWM            2
> >>   
> >> +#define SUNIV_GPE_UART0           5
> >>   #define SUN8I_GPE_TWI2           3
> >>   #define SUN50I_GPE_TWI2          3
> >>     
> >   

Reply via email to