From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Icenowy Zheng <icenowy@aosc.io>
Cc: Rob Herring <robh+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
Linus Walleij <linus.walleij@linaro.org>,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-sunxi@googlegroups.com
Subject: Re: [PATCH v2 4/6] clk: sunxi-ng: add support for the Allwinner H6 CCU
Date: Wed, 7 Feb 2018 19:49:59 +0100 [thread overview]
Message-ID: <20180207184959.5h233agyhva352s4@flea> (raw)
In-Reply-To: <3AB5A169-5EA5-4A45-95AF-4CA56F13A1CA@aosc.io>
[-- Attachment #1: Type: text/plain, Size: 5846 bytes --]
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@bootlin.com> 写到:
> >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@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.
> >
> >> +
> >> +#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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2018-02-07 18:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-03 15:49 [PATCH v2 0/6] Initial Allwinner H6 support Icenowy Zheng
2018-02-03 15:49 ` [PATCH v2 3/6] clk: sunxi-ng: Support fixed post-dividers on NKMP style clocks Icenowy Zheng
[not found] ` <20180203154942.63566-1-icenowy-h8G6r0blFSE@public.gmane.org>
2018-02-03 15:49 ` [PATCH v2 1/6] pinctrl: sunxi: support pin controllers with holes among IRQ banks Icenowy Zheng
[not found] ` <20180203154942.63566-2-icenowy-h8G6r0blFSE@public.gmane.org>
2018-02-07 8:51 ` Maxime Ripard
2018-02-03 15:49 ` [PATCH v2 2/6] pinctrl: sunxi: add support for the Allwinner H6 main pin controller Icenowy Zheng
[not found] ` <20180203154942.63566-3-icenowy-h8G6r0blFSE@public.gmane.org>
2018-02-08 20:16 ` Rob Herring
2018-02-13 0:38 ` [linux-sunxi] " André Przywara
2018-02-03 15:49 ` [PATCH v2 4/6] clk: sunxi-ng: add support for the Allwinner H6 CCU Icenowy Zheng
2018-02-07 9:02 ` Maxime Ripard
2018-02-07 9:11 ` Icenowy Zheng
2018-02-07 18:49 ` Maxime Ripard [this message]
2018-02-08 20:18 ` Rob Herring
2018-02-03 15:49 ` [PATCH v2 5/6] arm64: allwinner: h6: add the basical Allwinner H6 DTSI file Icenowy Zheng
2018-02-11 23:26 ` [linux-sunxi] " André Przywara
2018-02-12 9:16 ` Philippe Ombredanne
2018-02-03 15:49 ` [PATCH v2 6/6] arm64: allwinner: h6: add support for Pine H64 board Icenowy Zheng
[not found] ` <20180203154942.63566-7-icenowy-h8G6r0blFSE@public.gmane.org>
2018-02-11 23:26 ` André Przywara
2018-02-22 12:43 ` [PATCH v2 0/6] Initial Allwinner H6 support Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180207184959.5h233agyhva352s4@flea \
--to=maxime.ripard@bootlin.com \
--cc=devicetree@vger.kernel.org \
--cc=icenowy@aosc.io \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=robh+dt@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).