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

André Przywara
2020-12-02T23:08:12Z

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@arm.com>
>> ---
>> 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
(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@aosc.io>
>> + */
>> +
>> +#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
>>
>>
>>
>
>