On Wed, Feb 07, 2018 at 05:11:11PM +0800, Icenowy Zheng wrote:

于 2018年2月7日 GMT+08:00 下午5:02:10, Maxime Ripard [off-list ref] 写到:

quoted

Hi,

On Sat, Feb 03, 2018 at 11:49:40PM +0800, Icenowy Zheng wrote:

quoted

+	/* Force the output divider of video PLLs to 0 */
+	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]);
+	}

Why?

According to the manual the output divider here is
test only, and for daily use it should be 0.

quoted

quoted

+	/* Force OHCI 12M clock sources to 00 (12MHz divided from 48MHz) */
+	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]);
+	}

Why?

Just make them have a sure working parent. Why this
muxs exist is still mysterious.

quoted

quoted

+	/*
+	 * Force the post-divider of pll-video to 8 and the output divider

oh here's a typo -- it's audio, not video.

quoted

quoted

+	 * of it to 1.
+	 */
+	val = readl(reg + SUN50I_H6_PLL_AUDIO_REG);
+	val &= ~(GENMASK(21, 16) | BIT(0));
+	writel(val | (7 << 16), reg + SUN50I_H6_PLL_AUDIO_REG);

Why?

See the comments about pll-audio in the former sources :-)

You should have a comments for all those :)

quoted

quoted

diff --git a/include/dt-bindings/clock/sun50i-h6-ccu.h

b/include/dt-bindings/clock/sun50i-h6-ccu.h

quoted

new file mode 100644
index 000000000000..f7ddb241012c

--- /dev/null
+++ b/include/dt-bindings/clock/sun50i-h6-ccu.h

@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2017 Icenowy Zheng <icenowy@aosc.io>
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ or MIT)
+ */
+
+#ifndef _DT_BINDINGS_CLK_SUN50I_H6_H_
+#define _DT_BINDINGS_CLK_SUN50I_H6_H_
+
+#define CLK_PLL_PERIPH0		3

Why is that needed?

Parent of AR100 clock in PRCM.

ACK.

quoted

quoted

+
+#define CLK_CPUX		21
+
+#define CLK_APB1		26

Same thing here

As discussed in v1, the APB bus clock is used for PIO.

ACK

quoted

quoted

+
+#define CLK_DE			29
+#define CLK_BUS_DE		30
+#define CLK_DEINTERLACE		31
+#define CLK_BUS_DEINTERLACE	32
+#define CLK_GPU			33
+#define CLK_BUS_GPU		34
+#define CLK_CE			35
+#define CLK_BUS_CE		36
+#define CLK_VE			37
+#define CLK_BUS_VE		38
+#define CLK_EMCE		39
+#define CLK_BUS_EMCE		40
+#define CLK_VP9			41
+#define CLK_BUS_VP9		42
+#define CLK_BUS_DMA		43
+#define CLK_BUS_MSGBOX		44
+#define CLK_BUS_SPINLOCK	45
+#define CLK_BUS_HSTIMER		46
+#define CLK_AVS			47
+#define CLK_BUS_DBG		48
+#define CLK_BUS_PSI		49
+#define CLK_BUS_PWM		50
+#define CLK_BUS_IOMMU		51
+
+#define CLK_MBUS_DMA		53
+#define CLK_MBUS_VE		54
+#define CLK_MBUS_CE		55
+#define CLK_MBUS_TS		56
+#define CLK_MBUS_NAND		57
+#define CLK_MBUS_CSI		58
+#define CLK_MBUS_DEINTERLACE	59
+
+#define CLK_NAND0		61
+#define CLK_NAND1		62
+#define CLK_BUS_NAND		63
+#define CLK_MMC0		64
+#define CLK_MMC1		65
+#define CLK_MMC2		66
+#define CLK_BUS_MMC0		67
+#define CLK_BUS_MMC1		68
+#define CLK_BUS_MMC2		69
+#define CLK_BUS_UART0		70
+#define CLK_BUS_UART1		71
+#define CLK_BUS_UART2		72
+#define CLK_BUS_UART3		73
+#define CLK_BUS_I2C0		74
+#define CLK_BUS_I2C1		75
+#define CLK_BUS_I2C2		76
+#define CLK_BUS_I2C3		77
+#define CLK_BUS_SCR0		78
+#define CLK_BUS_SCR1		79
+#define CLK_SPI0		80
+#define CLK_SPI1		81
+#define CLK_BUS_SPI0		82
+#define CLK_BUS_SPI1		83
+#define CLK_BUS_EMAC		84
+#define CLK_TS			85
+#define CLK_BUS_TS		86
+#define CLK_IR_TX		87
+#define CLK_BUS_IR_TX		88
+#define CLK_BUS_THS		89
+#define CLK_I2S3		90
+#define CLK_I2S0		91
+#define CLK_I2S1		92
+#define CLK_I2S2		93
+#define CLK_BUS_I2S0		94
+#define CLK_BUS_I2S1		95
+#define CLK_BUS_I2S2		96
+#define CLK_BUS_I2S3		97
+#define CLK_SPDIF		98
+#define CLK_BUS_SPDIF		99
+#define CLK_DMIC		100
+#define CLK_BUS_DMIC		101
+#define CLK_AUDIO_HUB		102
+#define CLK_BUS_AUDIO_HUB	103
+#define CLK_USB_OHCI0		104
+#define CLK_USB_PHY0		105
+#define CLK_USB_PHY1		106
+#define CLK_USB_OHCI3		107
+#define CLK_USB_PHY3		108
+#define CLK_USB_HSIC_12M	109
+#define CLK_USB_HSIC		110
+#define CLK_BUS_OHCI0		111
+#define CLK_BUS_OHCI3		112
+#define CLK_BUS_EHCI0		113
+#define CLK_BUS_XHCI		114
+#define CLK_BUS_EHCI3		115
+#define CLK_BUS_OTG		116
+#define CLK_PCIE_REF_100M	117
+#define CLK_PCIE_REF		118
+#define CLK_PCIE_REF_OUT	119
+#define CLK_PCIE_MAXI		120
+#define CLK_PCIE_AUX		121

Dito.

I think AUX clock is used in PCIe code.

BTW the REF_100M clock is intermediate, and
does't need to be exported. Will fix it in the next version.

quoted

quoted

+#define CLK_BUS_PCIE		122
+#define CLK_HDMI		123
+#define CLK_HDMI_CEC		124
+#define CLK_BUS_HDMI		125
+#define CLK_BUS_TCON_TOP	126

Ditto.

TCON_TOP is not a clock parent -- it's a dedicated hw
part which controlls the connection between
DE2/3 mixers and TCONs.

Ok, great, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com