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

Andre Przywara
2022-01-26T02:06:07Z

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.

> +#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
>