mbox series

[0/6] clk: samsung: Introduce Exynos850 SoC clock driver

Message ID 20210914155607.14122-1-semen.protsenko@linaro.org
Headers show
Series clk: samsung: Introduce Exynos850 SoC clock driver | expand

Message

Sam Protsenko Sept. 14, 2021, 3:56 p.m. UTC 
This patch series provides the implementation for Exynos850 clock
driver, its documentation and corresponding changes for Samsung clock
infrastructure:
  - Adds new PLL types used in Exynos850 SoC, following TRM
  - Enables bus clock for each registered CMU, if it's provided

I tried to follow already established design for Samsung clock drivers
(getting most insights from Exynos7 and Exynos5433 clock drivers), and
integrate the driver in existing infrastructure. The whole driver was
implemented from scratch, using mostly TRM.

For now only basic clocks are implemented, including next blocks:
  - CMU_TOP
  - CMU_PERI
  - CMU_CORE
  - CMU_HSI

Some CMUs are still not implemented, but that can be added in future,
when the need arises. The driver also lacks CLKOUT support, PM ops and
automatic clocks control (using Q-Channel protocol). All that can be
added independently later.

Implemented clock tree was tested via UART and MMC drivers, and using
DebugFS clk support (e.g. using 'clk_summary' file). In order to keep
all clocks running I added 'clk_ignore_unused' kernel param in my local
tree, and defined CLOCK_ALLOW_WRITE_DEBUGFS in clk.c for actually
testing clocks via DebugFS.

Sam Protsenko (6):
  clk: samsung: Enable bus clock on init
  clk: samsung: clk-pll: Implement pll0822x PLL type
  clk: samsung: clk-pll: Implement pll0831x PLL type
  dt-bindings: clock: Add bindings definitions for Exynos850 CMU
  dt-bindings: clock: Document Exynos850 CMU bindings
  clk: samsung: Introduce Exynos850 clock driver

 .../clock/samsung,exynos850-clock.yaml        | 190 +++++
 drivers/clk/samsung/Makefile                  |   1 +
 drivers/clk/samsung/clk-exynos850.c           | 700 ++++++++++++++++++
 drivers/clk/samsung/clk-pll.c                 | 196 +++++
 drivers/clk/samsung/clk-pll.h                 |   2 +
 drivers/clk/samsung/clk.c                     |  13 +
 include/dt-bindings/clock/exynos850.h         |  72 ++
 7 files changed, 1174 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml
 create mode 100644 drivers/clk/samsung/clk-exynos850.c
 create mode 100644 include/dt-bindings/clock/exynos850.h

-- 
2.30.2

Comments

Krzysztof Kozlowski Sept. 15, 2021, 8:27 a.m. UTC | #1 
On 14/09/2021 17: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@linaro.org>

> ---

>  include/dt-bindings/clock/exynos850.h | 72 +++++++++++++++++++++++++++

>  1 file changed, 72 insertions(+)

>  create mode 100644 include/dt-bindings/clock/exynos850.h

> 



Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>



Best regards,
Krzysztof
Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics Sept. 15, 2021, 1:07 p.m. UTC | #2 
On 14.09.2021 17:56, Sam Protsenko wrote:
> +static void __init exynos850_cmu_top_init(struct device_node *np)

> +{

> +	exynos850_init_clocks(np, top_clk_regs, ARRAY_SIZE(top_clk_regs));

> +	samsung_cmu_register_one(np, &top_cmu_info);

> +}

> +

> +CLK_OF_DECLARE(exynos850_cmu_top, "samsung,exynos850-cmu-top",

> +	       exynos850_cmu_top_init);


Was there anything preventing you from making it a platform driver instead?

-- 
Regards,
Sylwester
Chanwoo Choi Sept. 15, 2021, 4:37 p.m. UTC | #3 
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.

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@linaro.org>

> ---

>   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@linaro.org>

> + *

> + * 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
Chanwoo Choi Sept. 15, 2021, 6:04 p.m. UTC | #4 
Hi Sam,

On 21. 9. 15. 오전 12:56, Sam Protsenko wrote:
> This is the initial implementation adding only basic clocks like UART,

> MMC, I2C and corresponding parent clocks. Design is influenced by

> Exynos7 and Exynos5433 clock drivers.

> 

> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>

> ---

>   drivers/clk/samsung/Makefile        |   1 +

>   drivers/clk/samsung/clk-exynos850.c | 700 ++++++++++++++++++++++++++++

>   2 files changed, 701 insertions(+)

>   create mode 100644 drivers/clk/samsung/clk-exynos850.c

> 

> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile

> index 028b2e27a37e..c46cf11e4d0b 100644

> --- a/drivers/clk/samsung/Makefile

> +++ b/drivers/clk/samsung/Makefile

> @@ -17,6 +17,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos5433.o

>   obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-exynos-audss.o

>   obj-$(CONFIG_EXYNOS_CLKOUT)	+= clk-exynos-clkout.o

>   obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7.o

> +obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos850.o

>   obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o

>   obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o

>   obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o

> diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c

> new file mode 100644

> index 000000000000..1028caa2102e

> --- /dev/null

> +++ b/drivers/clk/samsung/clk-exynos850.c

> @@ -0,0 +1,700 @@

> +// SPDX-License-Identifier: GPL-2.0-only

> +/*

> + * Copyright (C) 2021 Linaro Ltd.

> + * Author: Sam Protsenko <semen.protsenko@linaro.org>

> + *

> + * Common Clock Framework support for Exynos850 SoC.

> + */

> +

> +#include <linux/clk-provider.h>

> +#include <linux/of.h>

> +#include <linux/of_address.h>

> +

> +#include <dt-bindings/clock/exynos850.h>

> +

> +#include "clk.h"

> +

> +/* Gate register bits */

> +#define GATE_MANUAL		BIT(20)

> +#define GATE_ENABLE_HWACG	BIT(28)

> +

> +/* Gate register offsets range */

> +#define GATE_OFF_START		0x2000

> +#define GATE_OFF_END		0x2fff

> +

> +/**

> + * exynos850_init_clocks - Set clocks initial configuration

> + * @np:			CMU device tree node with "reg" property (CMU addr)

> + * @reg_offs:		Register offsets array for clocks to init

> + * @reg_offs_len:	Number of register offsets in reg_offs array

> + *

> + * Set manual control mode for all gate clocks.

> + */

> +static void __init exynos850_init_clocks(struct device_node *np,

> +		const unsigned long *reg_offs, size_t reg_offs_len)

> +{

> +	const __be32 *regaddr_p;

> +	u64 regaddr;

> +	u32 base;

> +	size_t i;

> +

> +	/* Get the base address ("reg" property in dts) */

> +	regaddr_p = of_get_address(np, 0, NULL, NULL);

> +	if (!regaddr_p)

> +		panic("%s: failed to get reg regaddr\n", __func__);

> +

> +	regaddr = of_translate_address(np, regaddr_p);

> +	if (regaddr == OF_BAD_ADDR || !regaddr)

> +		panic("%s: bad reg regaddr\n", __func__);

> +

> +	base = (u32)regaddr;

> +

> +	for (i = 0; i < reg_offs_len; ++i) {

> +		void __iomem *reg;

> +		u32 val;

> +

> +		/* Modify only gate clock registers */

> +		if (reg_offs[i] < GATE_OFF_START || reg_offs[i] > GATE_OFF_END)

> +			continue; > +

> +		reg = ioremap(base + reg_offs[i], 4);

> +		val = ioread32(reg);

> +		val |= GATE_MANUAL;

> +		val &= ~GATE_ENABLE_HWACG;

> +		iowrite32(val, reg);

> +		iounmap(reg);


I understand your intention for disabling HWACG.
But, it is not good to execute ioreamp/iounmap for each clock gate
register. I think that we need to consider the more pretty method
to initialize the clock register before clock registration.

[snip]

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi
Sam Protsenko Sept. 15, 2021, 10 p.m. UTC | #5 
On Wed, 15 Sept 2021 at 21:05, Chanwoo Choi <cwchoi00@gmail.com> wrote:
>
> Hi Sam,
>
> On 21. 9. 15. 오전 12:56, Sam Protsenko wrote:
> > This is the initial implementation adding only basic clocks like UART,
> > MMC, I2C and corresponding parent clocks. Design is influenced by
> > Exynos7 and Exynos5433 clock drivers.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> >   drivers/clk/samsung/Makefile        |   1 +
> >   drivers/clk/samsung/clk-exynos850.c | 700 ++++++++++++++++++++++++++++
> >   2 files changed, 701 insertions(+)
> >   create mode 100644 drivers/clk/samsung/clk-exynos850.c
> >
> > diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> > index 028b2e27a37e..c46cf11e4d0b 100644
> > --- a/drivers/clk/samsung/Makefile
> > +++ b/drivers/clk/samsung/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)       += clk-exynos5433.o
> >   obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) += clk-exynos-audss.o
> >   obj-$(CONFIG_EXYNOS_CLKOUT) += clk-exynos-clkout.o
> >   obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)       += clk-exynos7.o
> > +obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)        += clk-exynos850.o
> >   obj-$(CONFIG_S3C2410_COMMON_CLK)+= clk-s3c2410.o
> >   obj-$(CONFIG_S3C2410_COMMON_DCLK)+= clk-s3c2410-dclk.o
> >   obj-$(CONFIG_S3C2412_COMMON_CLK)+= clk-s3c2412.o
> > diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/clk-exynos850.c
> > new file mode 100644
> > index 000000000000..1028caa2102e
> > --- /dev/null
> > +++ b/drivers/clk/samsung/clk-exynos850.c
> > @@ -0,0 +1,700 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2021 Linaro Ltd.
> > + * Author: Sam Protsenko <semen.protsenko@linaro.org>
> > + *
> > + * Common Clock Framework support for Exynos850 SoC.
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +
> > +#include <dt-bindings/clock/exynos850.h>
> > +
> > +#include "clk.h"
> > +
> > +/* Gate register bits */
> > +#define GATE_MANUAL          BIT(20)
> > +#define GATE_ENABLE_HWACG    BIT(28)
> > +
> > +/* Gate register offsets range */
> > +#define GATE_OFF_START               0x2000
> > +#define GATE_OFF_END         0x2fff
> > +
> > +/**
> > + * exynos850_init_clocks - Set clocks initial configuration
> > + * @np:                      CMU device tree node with "reg" property (CMU addr)
> > + * @reg_offs:                Register offsets array for clocks to init
> > + * @reg_offs_len:    Number of register offsets in reg_offs array
> > + *
> > + * Set manual control mode for all gate clocks.
> > + */
> > +static void __init exynos850_init_clocks(struct device_node *np,
> > +             const unsigned long *reg_offs, size_t reg_offs_len)
> > +{
> > +     const __be32 *regaddr_p;
> > +     u64 regaddr;
> > +     u32 base;
> > +     size_t i;
> > +
> > +     /* Get the base address ("reg" property in dts) */
> > +     regaddr_p = of_get_address(np, 0, NULL, NULL);
> > +     if (!regaddr_p)
> > +             panic("%s: failed to get reg regaddr\n", __func__);
> > +
> > +     regaddr = of_translate_address(np, regaddr_p);
> > +     if (regaddr == OF_BAD_ADDR || !regaddr)
> > +             panic("%s: bad reg regaddr\n", __func__);
> > +
> > +     base = (u32)regaddr;
> > +
> > +     for (i = 0; i < reg_offs_len; ++i) {
> > +             void __iomem *reg;
> > +             u32 val;
> > +
> > +             /* Modify only gate clock registers */
> > +             if (reg_offs[i] < GATE_OFF_START || reg_offs[i] > GATE_OFF_END)
> > +                     continue; > +
> > +             reg = ioremap(base + reg_offs[i], 4);
> > +             val = ioread32(reg);
> > +             val |= GATE_MANUAL;
> > +             val &= ~GATE_ENABLE_HWACG;
> > +             iowrite32(val, reg);
> > +             iounmap(reg);
>
> I understand your intention for disabling HWACG.
> But, it is not good to execute ioreamp/iounmap for each clock gate
> register. I think that we need to consider the more pretty method
> to initialize the clock register before clock registration.
>
> [snip]
>

Hi guys,

Thanks for the quick review! I'll address all your comments once I get
back from vacation (in two weeks), and will send v2.

> --
> Best Regards,
> Samsung Electronics
> Chanwoo Choi
Sam Protsenko Oct. 5, 2021, 10:28 a.m. UTC | #6 
On Wed, 15 Sept 2021 at 19:37, Chanwoo Choi <cwchoi00@gmail.com> 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@linaro.org>

> > ---

> >   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@linaro.org>

> > + *

> > + * 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
Sam Protsenko Oct. 5, 2021, 11:36 a.m. UTC | #7 
On Wed, 15 Sept 2021 at 16:07, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>

> On 14.09.2021 17:56, Sam Protsenko wrote:

> > +static void __init exynos850_cmu_top_init(struct device_node *np)

> > +{

> > +     exynos850_init_clocks(np, top_clk_regs, ARRAY_SIZE(top_clk_regs));

> > +     samsung_cmu_register_one(np, &top_cmu_info);

> > +}

> > +

> > +CLK_OF_DECLARE(exynos850_cmu_top, "samsung,exynos850-cmu-top",

> > +            exynos850_cmu_top_init);

>

> Was there anything preventing you from making it a platform driver instead?

>


Can you please elaborate on benefits of adding platform driver? I
don't implement PM ops for now, and I can see that clk-exynos7.c does
not add platform driver as well... clk-exynos5433.c seems to use
platform_driver for PM ops only.

> --

> Regards,

> Sylwester
Krzysztof Kozlowski Oct. 6, 2021, 12:46 p.m. UTC | #8 
On 05/10/2021 13:36, Sam Protsenko wrote:
> On Wed, 15 Sept 2021 at 16:07, Sylwester Nawrocki

> <s.nawrocki@samsung.com> wrote:

>>

>> On 14.09.2021 17:56, Sam Protsenko wrote:

>>> +static void __init exynos850_cmu_top_init(struct device_node *np)

>>> +{

>>> +     exynos850_init_clocks(np, top_clk_regs, ARRAY_SIZE(top_clk_regs));

>>> +     samsung_cmu_register_one(np, &top_cmu_info);

>>> +}

>>> +

>>> +CLK_OF_DECLARE(exynos850_cmu_top, "samsung,exynos850-cmu-top",

>>> +            exynos850_cmu_top_init);

>>

>> Was there anything preventing you from making it a platform driver instead?

>>

> 

> Can you please elaborate on benefits of adding platform driver? I

> don't implement PM ops for now, and I can see that clk-exynos7.c does

> not add platform driver as well... clk-exynos5433.c seems to use

> platform_driver for PM ops only.


I said it in response to patch 1, so just for the record:
Exynos7 is not the example you are looking for. :) Exynos5433 is.


Best regards,
Krzysztof