On Wed, Feb 07, 2018 at 05:11:11PM +0800, Icenowy Zheng wrote:
>
>
> 于 2018年2月7日 GMT+08:00 下午5:02:10, Maxime Ripard <maxime.ripard@xxxxxxxxxxx> 写到:
> >Hi,
> >
> >On Sat, Feb 03, 2018 at 11:49:40PM +0800, Icenowy Zheng wrote:
> >> + /* 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.
>
> >
> >> + /* 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.
>
> >
> >> + /*
> >> + * Force the post-divider of pll-video to 8 and the output divider
>
> oh here's a typo -- it's audio, not video.
>
> >> + * 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 :)
>
> >> diff --git a/include/dt-bindings/clock/sun50i-h6-ccu.h
> >b/include/dt-bindings/clock/sun50i-h6-ccu.h
> >> 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@xxxxxxx>
> >> + *
> >> + * 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.
> >
> >> +
> >> +#define CLK_CPUX 21
> >> +
> >> +#define CLK_APB1 26
> >
> >Same thing here
>
> As discussed in v1, the APB bus clock is used for PIO.
ACK
> >
> >> +
> >> +#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.
>
> >
> >> +#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
Attachment:
signature.asc
Description: PGP signature
