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?

> +     /* 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?

> +     /*
> +      * Force the post-divider of pll-video to 8 and the output divider
> +      * 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?
> 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 <[email protected]>
> + *
> + * 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?

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

Same thing here

> +
> +#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.

> +#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.

Remember, you should only expose here the clocks that devices are
going to use, and remove as much intermediate clocks as possible.

If you're unsure, keep them out of the dt-binding include, we can
always move one to the dt-binding header, while the other way isn't
possible.

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

Attachment: signature.asc
Description: PGP signature

Reply via email to