On Wed, 15 Sept 2021 at 19:37, Chanwoo Choi <cwchoi00@xxxxxxxxx> wrote: > > Hi, > > You don't add clock ids for the all defined clocks in clk-exynos850.c. > I recommend that add all clock ids for the defined clocks if possible. > > If you want to change the parent clock of mux or change the clock rate > of div rate for some clocks, you have to touch the files as following: > - include/dt-bindings/clock/exynos850.h > - drivers/clk/samsung/clk-exynos850.c > - exynos850 dt files > > If you define the clock ids for all clocks added to this patchset, > you can change the parent or rate by just editing the dt files. > Hi Chanwoo, I see your point. But I have intentionally omitted some clock ids, which can't be / shouldn't be used by consumers in device tree. Actually I took that idea from clk-exynos7.c. Krzysztof, Sylwester: can you please advice if all clock ids should be defined, or only those that are going to be used in dts clk consumers? I don't mind reworking the patch, just want to be sure which design approach we want to follow. Thanks! > But, I have no strongly objection about just keeping this patch. > > > On 21. 9. 15. 오전 12:56, Sam Protsenko wrote: > > Clock controller driver is designed to have separate instances for each > > particular CMU. So clock IDs in this bindings header also start from 1 > > for each CMU. > > > > Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx> > > --- > > include/dt-bindings/clock/exynos850.h | 72 +++++++++++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > create mode 100644 include/dt-bindings/clock/exynos850.h > > > > diff --git a/include/dt-bindings/clock/exynos850.h b/include/dt-bindings/clock/exynos850.h > > new file mode 100644 > > index 000000000000..2f0a7f619627 > > --- /dev/null > > +++ b/include/dt-bindings/clock/exynos850.h > > @@ -0,0 +1,72 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Copyright (C) 2021 Linaro Ltd. > > + * Author: Sam Protsenko <semen.protsenko@xxxxxxxxxx> > > + * > > + * Device Tree binding constants for Exynos850 clock controller. > > + */ > > + > > +#ifndef _DT_BINDINGS_CLOCK_EXYNOS_850_H > > +#define _DT_BINDINGS_CLOCK_EXYNOS_850_H > > + > > +/* CMU_TOP */ > > +#define DOUT_HSI_BUS 1 > > +#define DOUT_HSI_MMC_CARD 2 > > +#define DOUT_HSI_USB20DRD 3 > > +#define DOUT_PERI_BUS 4 > > +#define DOUT_PERI_UART 5 > > +#define DOUT_PERI_IP 6 > > +#define DOUT_CORE_BUS 7 > > +#define DOUT_CORE_CCI 8 > > +#define DOUT_CORE_MMC_EMBD 9 > > +#define DOUT_CORE_SSS 10 > > +#define TOP_NR_CLK 11 > > + > > +/* CMU_HSI */ > > +#define GOUT_USB_RTC_CLK 1 > > +#define GOUT_USB_REF_CLK 2 > > +#define GOUT_USB_PHY_REF_CLK 3 > > +#define GOUT_USB_PHY_ACLK 4 > > +#define GOUT_USB_BUS_EARLY_CLK 5 > > +#define GOUT_GPIO_HSI_PCLK 6 > > +#define GOUT_MMC_CARD_ACLK 7 > > +#define GOUT_MMC_CARD_SDCLKIN 8 > > +#define GOUT_SYSREG_HSI_PCLK 9 > > +#define HSI_NR_CLK 10 > > + > > +/* CMU_PERI */ > > +#define GOUT_GPIO_PERI_PCLK 1 > > +#define GOUT_HSI2C0_IPCLK 2 > > +#define GOUT_HSI2C0_PCLK 3 > > +#define GOUT_HSI2C1_IPCLK 4 > > +#define GOUT_HSI2C1_PCLK 5 > > +#define GOUT_HSI2C2_IPCLK 6 > > +#define GOUT_HSI2C2_PCLK 7 > > +#define GOUT_I2C0_PCLK 8 > > +#define GOUT_I2C1_PCLK 9 > > +#define GOUT_I2C2_PCLK 10 > > +#define GOUT_I2C3_PCLK 11 > > +#define GOUT_I2C4_PCLK 12 > > +#define GOUT_I2C5_PCLK 13 > > +#define GOUT_I2C6_PCLK 14 > > +#define GOUT_MCT_PCLK 15 > > +#define GOUT_PWM_MOTOR_PCLK 16 > > +#define GOUT_SPI0_IPCLK 17 > > +#define GOUT_SPI0_PCLK 18 > > +#define GOUT_SYSREG_PERI_PCLK 19 > > +#define GOUT_UART_IPCLK 20 > > +#define GOUT_UART_PCLK 21 > > +#define GOUT_WDT0_PCLK 22 > > +#define GOUT_WDT1_PCLK 23 > > +#define PERI_NR_CLK 24 > > + > > +/* CMU_CORE */ > > +#define GOUT_CCI_ACLK 1 > > +#define GOUT_GIC_CLK 2 > > +#define GOUT_MMC_EMBD_ACLK 3 > > +#define GOUT_MMC_EMBD_SDCLKIN 4 > > +#define GOUT_SSS_ACLK 5 > > +#define GOUT_SSS_PCLK 6 > > +#define CORE_NR_CLK 7 > > + > > +#endif /* _DT_BINDINGS_CLOCK_EXYNOS_850_H */ > > > > > -- > Best Regards, > Samsung Electronics > Chanwoo Choi
