Re: [linux-sunxi] Re: [PATCH 5/8] clk: sunxi-ng: Add support for the Allwinner H616 CCU

From: Icenowy Zheng
Date: Sat Dec 05 2020 - 12:02:31 EST


在 2020-12-02星期三的 23:06 +0000,André Przywara写道:
> On 02/12/2020 21:03, Jernej Škrabec wrote:
> > Dne sreda, 02. december 2020 ob 14:54:06 CET je Andre Przywara
> > napisal(a):
> > > While the clocks are fairly similar to the H6, many differ in
> > > tiny
> > > details, so a separate clock driver seems indicated.
> > >
> > > Derived from the H6 clock driver, and adjusted according to the
> > > manual.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > > ---
> > > drivers/clk/sunxi-ng/Kconfig | 7 +-
> > > drivers/clk/sunxi-ng/Makefile | 1 +
> > > drivers/clk/sunxi-ng/ccu-sun50i-h616.c | 1134
> > > +++++++++++++++++++
> > > drivers/clk/sunxi-ng/ccu-sun50i-h616.h | 58 +
> > > include/dt-bindings/clock/sun50i-h616-ccu.h | 110 ++
> > > include/dt-bindings/reset/sun50i-h616-ccu.h | 67 ++
> > > 6 files changed, 1376 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h616.c
> > > create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h616.h
> > > create mode 100644 include/dt-bindings/clock/sun50i-h616-ccu.h
> > > create mode 100644 include/dt-bindings/reset/sun50i-h616-ccu.h
> > >
> > > diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-
> > > ng/Kconfig
> > > index ce5f5847d5d3..cd46d8853876 100644
> > > --- a/drivers/clk/sunxi-ng/Kconfig
> > > +++ b/drivers/clk/sunxi-ng/Kconfig
>
> ....
>
> > > +static int sun50i_h616_ccu_probe(struct platform_device *pdev)
> > > +{
> > > + struct resource *res;
> > > + void __iomem *reg;
> > > + u32 val;
> > > + int i;
> > > +
> > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > + reg = devm_ioremap_resource(&pdev->dev, res);
> > > + if (IS_ERR(reg))
> > > + return PTR_ERR(reg);
> > > +
> > > + /* Enable the lock bits on all PLLs */
> > > + for (i = 0; i < ARRAY_SIZE(pll_regs); i++) {
> > > + val = readl(reg + pll_regs[i]);
> > > + val |= BIT(29);
> >
> > You should also add BIT(27) here. It enables PLL output (new
> > functionality in
> > H616). Without this only clocks which are child of PLL_CPUX and
> > PLL_PERIPH0
> > would work (set up by U-Boot). I'm pretty sure that's not intended
> > usage but
> > until we know better, it's ok imo.
>
> Ah right, the output enable bit. I was wondering if the A100 solution
> would better here: use bit 27 as the .enable value, and actually
> enable

Why not .enable = BIT(27) | BIT(31) ?

> (bit31) all PLLs here.
> Or we add another field, maybe a flag, to allow the kernel to decide
> which bit to use. Clement suggested something like that on IRC.
> But for now I can surely just set bit 27 here as well.
>
> > > + writel(val, reg + pll_regs[i]);
> > > + }
> > > +
> > > + /*
> > > + * Force the output divider of video PLLs to 0.
> > > + *
> > > + * See the comment before pll-video0 definition for the reason.
> > > + */
> > > + for (i = 0; i < ARRAY_SIZE(pll_video_regs); i++) {
> > > + val = readl(reg + pll_video_regs[i]);
> > > + val &= ~BIT(0);
> > > + writel(val, reg + pll_video_regs[i]);
> > > + }
> > > +
> > > + /*
> > > + * Force OHCI 12M clock sources to 00 (12MHz divided from
> > > 48MHz)
> > > + *
> > > + * This clock mux is still mysterious, and the code just
> > > enforces
> > > + * it to have a valid clock parent.
> > > + */
> > > + for (i = 0; i < ARRAY_SIZE(usb2_clk_regs); i++) {
> > > + val = readl(reg + usb2_clk_regs[i]);
> > > + val &= ~GENMASK(25, 24);
> > > + writel (val, reg + usb2_clk_regs[i]);
> > > + }
> > > +
> > > + /*
> > > + * Force the post-divider of pll-audio to 12 and the output
> > > divider
> > > + * of it to 2, so 24576000 and 22579200 rates can be set
> > > exactly.
> > > + */
> > > + val = readl(reg + SUN50I_H616_PLL_AUDIO_REG);
> > > + val &= ~(GENMASK(21, 16) | BIT(0));
> > > + writel(val | (11 << 16) | BIT(0), reg +
> > > SUN50I_H616_PLL_AUDIO_REG);
> > > +
> > > + /*
> > > + * First clock parent (osc32K) is unusable for CEC. But since
> > > there
> > > + * is no good way to force parent switch (both run with same
> > > frequency),
> > > + * just set second clock parent here.
> > > + */
> > > + val = readl(reg + SUN50I_H616_HDMI_CEC_CLK_REG);
> > > + val |= BIT(24);
> > > + writel(val, reg + SUN50I_H616_HDMI_CEC_CLK_REG);
> > > +
> > > + return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > &sun50i_h616_ccu_desc);
> > > +}
> > > +
> > > +static const struct of_device_id sun50i_h616_ccu_ids[] = {
> > > + { .compatible = "allwinner,sun50i-h616-ccu",
> > > + .data = &sun50i_h616_ccu_desc },
> > > + { }
> > > +};
> > > +
> > > +static struct platform_driver sun50i_h616_ccu_driver = {
> > > + .probe = sun50i_h616_ccu_probe,
> > > + .driver = {
> > > + .name = "sun50i-h616-ccu",
> > > + .of_match_table = sun50i_h616_ccu_ids,
> > > + },
> > > +};
> > > +builtin_platform_driver(sun50i_h616_ccu_driver);
> >
> > Please use CLK_OF_DECLARE() instead. That way clocks will be
> > initialized
> > earlier and it will be actually possible to register both timer
> > peripherals
> > (once DT nodes are added). If pdev or dev is ever needed, two
> > stage
> > initialization can be made later.
>
> Sure, will do.
>
> Thanks for having a look!
>
> Andre
>
> > Best regards,
> > Jernej
> >
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h616.h
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-h616.h
> > > new file mode 100644
> > > index 000000000000..da8f0b1206f9
> > > --- /dev/null
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h616.h
> > > @@ -0,0 +1,58 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright 2020 Arm Ltd.
> > > + */
> > > +
> > > +#ifndef _CCU_SUN50I_H616_H_
> > > +#define _CCU_SUN50I_H616_H_
> > > +
> > > +#include <dt-bindings/clock/sun50i-h616-ccu.h>
> > > +#include <dt-bindings/reset/sun50i-h616-ccu.h>
> > > +
> > > +#define CLK_OSC12M 0
> > > +#define CLK_PLL_CPUX 1
> > > +#define CLK_PLL_DDR0 2
> > > +#define CLK_PLL_DDR1 3
> > > +
> > > +/* PLL_PERIPH0 exported for PRCM */
> > > +
> > > +#define CLK_PLL_PERIPH0_2X 5
> > > +#define CLK_PLL_PERIPH0_4X 6
> > > +#define CLK_PLL_PERIPH1 7
> > > +#define CLK_PLL_PERIPH1_2X 8
> > > +#define CLK_PLL_PERIPH1_4X 9
> > > +#define CLK_PLL_GPU 10
> > > +#define CLK_PLL_VIDEO0 11
> > > +#define CLK_PLL_VIDEO0_4X 12
> > > +#define CLK_PLL_VIDEO1 13
> > > +#define CLK_PLL_VIDEO1_4X 14
> > > +#define CLK_PLL_VIDEO2 15
> > > +#define CLK_PLL_VIDEO2_4X 16
> > > +#define CLK_PLL_VE 17
> > > +#define CLK_PLL_DE 18
> > > +#define CLK_PLL_AUDIO_HS 19
> > > +#define CLK_PLL_AUDIO_1X 20
> > > +#define CLK_PLL_AUDIO_2X 21
> > > +#define CLK_PLL_AUDIO_4X 22
> > > +
> > > +/* CPUX clock exported for DVFS */
> > > +
> > > +#define CLK_AXI 24
> > > +#define CLK_CPUX_APB 25
> > > +#define CLK_PSI_AHB1_AHB2 26
> > > +#define CLK_AHB3 27
> > > +
> > > +/* APB1 clock exported for PIO */
> > > +
> > > +#define CLK_APB2 29
> > > +#define CLK_MBUS 30
> > > +
> > > +/* All module clocks and bus gates are exported except DRAM */
> > > +
> > > +#define CLK_DRAM 51
> > > +
> > > +#define CLK_BUS_DRAM 58
> > > +
> > > +#define CLK_NUMBER (CLK_BUS_HDCP + 1)
> > > +
> > > +#endif /* _CCU_SUN50I_H616_H_ */
> > > diff --git a/include/dt-bindings/clock/sun50i-h616-ccu.h
> > > b/include/dt-bindings/clock/sun50i-h616-ccu.h
> > > new file mode 100644
> > > index 000000000000..a9cc8844e3a9
> > > --- /dev/null
> > > +++ b/include/dt-bindings/clock/sun50i-h616-ccu.h
> > > @@ -0,0 +1,110 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > +/*
> > > + * Copyright (C) 2020 Arm Ltd.
> > > + */
> > > +
> > > +#ifndef _DT_BINDINGS_CLK_SUN50I_H616_H_
> > > +#define _DT_BINDINGS_CLK_SUN50I_H616_H_
> > > +
> > > +#define CLK_PLL_PERIPH0 4
> > > +
> > > +#define CLK_CPUX 23
> > > +
> > > +#define CLK_APB1 28
> > > +
> > > +#define CLK_DE 31
> > > +#define CLK_BUS_DE 32
> > > +#define CLK_DEINTERLACE 33
> > > +#define CLK_BUS_DEINTERLACE 34
> > > +#define CLK_G2D 35
> > > +#define CLK_BUS_G2D 36
> > > +#define CLK_GPU0 37
> > > +#define CLK_BUS_GPU 38
> > > +#define CLK_GPU1 39
> > > +#define CLK_CE 40
> > > +#define CLK_BUS_CE 41
> > > +#define CLK_VE 42
> > > +#define CLK_BUS_VE 43
> > > +#define CLK_BUS_DMA 44
> > > +#define CLK_BUS_HSTIMER 45
> > > +#define CLK_AVS 46
> > > +#define CLK_BUS_DBG 47
> > > +#define CLK_BUS_PSI 48
> > > +#define CLK_BUS_PWM 49
> > > +#define CLK_BUS_IOMMU 50
> > > +
> > > +#define CLK_MBUS_DMA 52
> > > +#define CLK_MBUS_VE 53
> > > +#define CLK_MBUS_CE 54
> > > +#define CLK_MBUS_TS 55
> > > +#define CLK_MBUS_NAND 56
> > > +#define CLK_MBUS_G2D 57
> > > +
> > > +#define CLK_NAND0 59
> > > +#define CLK_NAND1 60
> > > +#define CLK_BUS_NAND 61
> > > +#define CLK_MMC0 62
> > > +#define CLK_MMC1 63
> > > +#define CLK_MMC2 64
> > > +#define CLK_BUS_MMC0 65
> > > +#define CLK_BUS_MMC1 66
> > > +#define CLK_BUS_MMC2 67
> > > +#define CLK_BUS_UART0 68
> > > +#define CLK_BUS_UART1 69
> > > +#define CLK_BUS_UART2 70
> > > +#define CLK_BUS_UART3 71
> > > +#define CLK_BUS_UART4 72
> > > +#define CLK_BUS_UART5 73
> > > +#define CLK_BUS_I2C0 74
> > > +#define CLK_BUS_I2C1 75
> > > +#define CLK_BUS_I2C2 76
> > > +#define CLK_BUS_I2C3 77
> > > +#define CLK_BUS_I2C4 78
> > > +#define CLK_SPI0 79
> > > +#define CLK_SPI1 80
> > > +#define CLK_BUS_SPI0 81
> > > +#define CLK_BUS_SPI1 82
> > > +#define CLK_EMAC_25M 83
> > > +#define CLK_BUS_EMAC0 84
> > > +#define CLK_BUS_EMAC1 85
> > > +#define CLK_TS 86
> > > +#define CLK_BUS_TS 87
> > > +#define CLK_BUS_THS 88
> > > +#define CLK_SPDIF 89
> > > +#define CLK_BUS_SPDIF 90
> > > +#define CLK_DMIC 91
> > > +#define CLK_BUS_DMIC 92
> > > +#define CLK_AUDIO_CODEC_1X 93
> > > +#define CLK_AUDIO_CODEC_4X 94
> > > +#define CLK_BUS_AUDIO_CODEC 95
> > > +#define CLK_AUDIO_HUB 96
> > > +#define CLK_BUS_AUDIO_HUB 97
> > > +#define CLK_USB_OHCI0 98
> > > +#define CLK_USB_PHY0 99
> > > +#define CLK_USB_OHCI1 100
> > > +#define CLK_USB_PHY1 101
> > > +#define CLK_USB_OHCI2 102
> > > +#define CLK_USB_PHY2 103
> > > +#define CLK_USB_OHCI3 104
> > > +#define CLK_USB_PHY3 105
> > > +#define CLK_BUS_OHCI0 106
> > > +#define CLK_BUS_OHCI1 107
> > > +#define CLK_BUS_OHCI2 108
> > > +#define CLK_BUS_OHCI3 109
> > > +#define CLK_BUS_EHCI0 110
> > > +#define CLK_BUS_EHCI1 111
> > > +#define CLK_BUS_EHCI2 112
> > > +#define CLK_BUS_EHCI3 113
> > > +#define CLK_BUS_OTG 114
> > > +#define CLK_BUS_KEYADC 115
> > > +#define CLK_HDMI 116
> > > +#define CLK_HDMI_SLOW 117
> > > +#define CLK_HDMI_CEC 118
> > > +#define CLK_BUS_HDMI 119
> > > +#define CLK_BUS_TCON_TOP 120
> > > +#define CLK_TCON_TV0 121
> > > +#define CLK_BUS_TCON_TV0 122
> > > +#define CLK_HDCP 123
> > > +#define CLK_BUS_HDCP 124
> > > +
> > > +#endif /* _DT_BINDINGS_CLK_SUN50I_H616_H_ */
> > > diff --git a/include/dt-bindings/reset/sun50i-h616-ccu.h
> > > b/include/dt-bindings/reset/sun50i-h616-ccu.h
> > > new file mode 100644
> > > index 000000000000..1c992cfbbbab
> > > --- /dev/null
> > > +++ b/include/dt-bindings/reset/sun50i-h616-ccu.h
> > > @@ -0,0 +1,67 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > +/*
> > > + * Copyright (C) 2017 Icenowy Zheng <icenowy@xxxxxxx>
> > > + */
> > > +
> > > +#ifndef _DT_BINDINGS_RESET_SUN50I_H616_H_
> > > +#define _DT_BINDINGS_RESET_SUN50I_H616_H_
> > > +
> > > +#define RST_MBUS 0
> > > +#define RST_BUS_DE 1
> > > +#define RST_BUS_DEINTERLACE 2
> > > +#define RST_BUS_GPU 3
> > > +#define RST_BUS_CE 4
> > > +#define RST_BUS_VE 5
> > > +#define RST_BUS_DMA 6
> > > +#define RST_BUS_HSTIMER 7
> > > +#define RST_BUS_DBG 8
> > > +#define RST_BUS_PSI 9
> > > +#define RST_BUS_PWM 10
> > > +#define RST_BUS_IOMMU 11
> > > +#define RST_BUS_DRAM 12
> > > +#define RST_BUS_NAND 13
> > > +#define RST_BUS_MMC0 14
> > > +#define RST_BUS_MMC1 15
> > > +#define RST_BUS_MMC2 16
> > > +#define RST_BUS_UART0 17
> > > +#define RST_BUS_UART1 18
> > > +#define RST_BUS_UART2 19
> > > +#define RST_BUS_UART3 20
> > > +#define RST_BUS_UART4 21
> > > +#define RST_BUS_UART5 22
> > > +#define RST_BUS_I2C0 23
> > > +#define RST_BUS_I2C1 24
> > > +#define RST_BUS_I2C2 25
> > > +#define RST_BUS_I2C3 26
> > > +#define RST_BUS_I2C4 27
> > > +#define RST_BUS_SPI0 28
> > > +#define RST_BUS_SPI1 29
> > > +#define RST_BUS_EMAC0 30
> > > +#define RST_BUS_EMAC1 31
> > > +#define RST_BUS_TS 32
> > > +#define RST_BUS_THS 33
> > > +#define RST_BUS_SPDIF 34
> > > +#define RST_BUS_DMIC 35
> > > +#define RST_BUS_AUDIO_CODEC 36
> > > +#define RST_BUS_AUDIO_HUB 37
> > > +#define RST_USB_PHY0 38
> > > +#define RST_USB_PHY1 39
> > > +#define RST_USB_PHY2 40
> > > +#define RST_USB_PHY3 41
> > > +#define RST_BUS_OHCI0 42
> > > +#define RST_BUS_OHCI1 43
> > > +#define RST_BUS_OHCI2 44
> > > +#define RST_BUS_OHCI3 45
> > > +#define RST_BUS_EHCI0 46
> > > +#define RST_BUS_EHCI1 47
> > > +#define RST_BUS_EHCI2 48
> > > +#define RST_BUS_EHCI3 49
> > > +#define RST_BUS_OTG 50
> > > +#define RST_BUS_HDMI 51
> > > +#define RST_BUS_HDMI_SUB 52
> > > +#define RST_BUS_TCON_TOP 53
> > > +#define RST_BUS_TCON_TV0 54
> > > +#define RST_BUS_HDCP 55
> > > +#define RST_BUS_KEYADC 56
> > > +
> > > +#endif /* _DT_BINDINGS_RESET_SUN50I_H616_H_ */
> > > --
> > > 2.17.5
> > >
> > >
> > >