Re: [PATCH 05/11] ARM: sunxi: Add support for F1C100s

Andre Przywara
2022-01-26T10:08:41Z

On Tue, 25 Jan 2022 23:53:27 -0500
Jesse Taube <mr.bossman075@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.bossman075@gmail.com> wrote:
> >
> > Hi,
> >
> > can you please change the subject, to maybe mention clocks? Reads
> > rather generic as is.
> >
> >> From: Icenowy Zheng <icenowy@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 <icenowy@aosc.io>
> >> Signed-off-by: Jesse Taube <Mr.Bossman075@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
> >>
> >