[10/12] clk: samsung: Add Exynos850 clock driver stub

For now it's just a stub driver to make serial driver work. Later it
will be implemented properly.

This driver doesn't really change clocks, only registers the UART clock
as a fixed-rate clock. Without this clock driver the UART driver won't
work, as it's trying to obtain "uart" clock and fails if it's not able
to. From drivers/tty/serial/samsung_tty.c:

8<------------------------------------------------------------------->8
    ourport->clk = clk_get(&platdev->dev, "uart");
    if (IS_ERR(ourport->clk)) {
        pr_err("%s: Controller clock not found\n",
                dev_name(&platdev->dev));
        ret = PTR_ERR(ourport->clk);
        goto err;
    }
8<------------------------------------------------------------------->8

In order to get functional serial console we have to implement that
minimal clock driver with "uart" clock. It's not necessary to actually
configure clocks, as those are already configured in bootloader, so
kernel can rely on that for now.

80 column limit is broken here to make checkpatch happy, otherwise it
swears about incorrect __initconst usage.

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

---
 drivers/clk/samsung/Makefile        |  1 +
 drivers/clk/samsung/clk-exynos850.c | 63 +++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 drivers/clk/samsung/clk-exynos850.c

-- 
2.30.2

On Fri, Jul 30, 2021 at 5:51 PM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> For now it's just a stub driver to make serial driver work. Later it

make the serial

> will be implemented properly.
>
> This driver doesn't really change clocks, only registers the UART clock
> as a fixed-rate clock. Without this clock driver the UART driver won't
> work, as it's trying to obtain "uart" clock and fails if it's not able
> to.


> From drivers/tty/serial/samsung_tty.c:
>
> 8<------------------------------------------------------------------->8
>     ourport->clk = clk_get(&platdev->dev, "uart");
>     if (IS_ERR(ourport->clk)) {
>         pr_err("%s: Controller clock not found\n",
>                 dev_name(&platdev->dev));
>         ret = PTR_ERR(ourport->clk);
>         goto err;
>     }
> 8<------------------------------------------------------------------->8

This is not needed in the commit message.

> In order to get functional serial console we have to implement that

get a functional

> minimal clock driver with "uart" clock. It's not necessary to actually
> configure clocks, as those are already configured in bootloader, so
> kernel can rely on that for now.

> 80 column limit is broken here to make checkpatch happy, otherwise it
> swears about incorrect __initconst usage.

Again, no need to be in the commit message, use the comment field for
this (after the cutter '---' line below).


> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/clk/samsung/Makefile        |  1 +
>  drivers/clk/samsung/clk-exynos850.c | 63 +++++++++++++++++++++++++++++
>  2 files changed, 64 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..3192ec9bb90b
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos850.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
> + * Copyright (C) 2021 Linaro Ltd.
> + *
> + * Common Clock Framework support for Exynos850 SoC.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

+ blank line?

> +#include <dt-bindings/clock/exynos850.h>
> +
> +#include "clk.h"
> +
> +/* Fixed rate clocks generated outside the SoC */
> +static struct samsung_fixed_rate_clock exynos850_fixed_rate_ext_clks[] __initdata = {
> +       FRATE(OSCCLK, "fin_pll", NULL, 0, 26000000),
> +};
> +
> +/*
> + * Model the UART clock as a fixed-rate clock for now, to make serial driver
> + * work. This clock is already configured in the bootloader.
> + */
> +static const struct samsung_fixed_rate_clock exynos850_peri_clks[] __initconst = {
> +       FRATE(DOUT_UART, "DOUT_UART", NULL, 0, 200000000),
> +};
> +
> +static const struct of_device_id ext_clk_match[] __initconst = {

> +       { .compatible = "samsung,exynos850-oscclk", .data = (void *)0 },

0 is the default for static variables.

> +       {},

No comma needed for the terminator lines.

> +};
> +
> +void __init exynos850_clk_init(struct device_node *np)
> +{
> +       void __iomem *reg_base;
> +       struct samsung_clk_provider *ctx;

> +       if (!np)
> +               panic("%s: unable to determine soc\n", __func__);

Check for the sake of additional code?

> +       reg_base = of_iomap(np, 0);

This will fail when np == NULL.

> +       if (!reg_base)
> +               panic("%s: failed to map registers\n", __func__);
> +
> +       ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
> +       if (!ctx)
> +               panic("%s: unable to allocate ctx\n", __func__);
> +
> +       samsung_clk_of_register_fixed_ext(ctx,
> +                       exynos850_fixed_rate_ext_clks,
> +                       ARRAY_SIZE(exynos850_fixed_rate_ext_clks),
> +                       ext_clk_match);
> +
> +       samsung_clk_register_fixed_rate(ctx, exynos850_peri_clks,
> +                       ARRAY_SIZE(exynos850_peri_clks));
> +
> +       samsung_clk_of_add_provider(np, ctx);
> +}
> +
> +CLK_OF_DECLARE(exynos850_clk, "samsung,exynos850-clock", exynos850_clk_init);

Hi Andy,

On Fri, 30 Jul 2021 at 18:12, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Fri, Jul 30, 2021 at 5:51 PM Sam Protsenko
> <semen.protsenko@linaro.org> wrote:
> >
> > For now it's just a stub driver to make serial driver work. Later it
>
> make the serial
>
> > will be implemented properly.
> >
> > This driver doesn't really change clocks, only registers the UART clock
> > as a fixed-rate clock. Without this clock driver the UART driver won't
> > work, as it's trying to obtain "uart" clock and fails if it's not able
> > to.
>
>
> > From drivers/tty/serial/samsung_tty.c:
> >
> > 8<------------------------------------------------------------------->8
> >     ourport->clk = clk_get(&platdev->dev, "uart");
> >     if (IS_ERR(ourport->clk)) {
> >         pr_err("%s: Controller clock not found\n",
> >                 dev_name(&platdev->dev));
> >         ret = PTR_ERR(ourport->clk);
> >         goto err;
> >     }
> > 8<------------------------------------------------------------------->8
>
> This is not needed in the commit message.
>
> > In order to get functional serial console we have to implement that
>
> get a functional
>
> > minimal clock driver with "uart" clock. It's not necessary to actually
> > configure clocks, as those are already configured in bootloader, so
> > kernel can rely on that for now.
>
> > 80 column limit is broken here to make checkpatch happy, otherwise it
> > swears about incorrect __initconst usage.
>
> Again, no need to be in the commit message, use the comment field for
> this (after the cutter '---' line below).
>
>
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> >  drivers/clk/samsung/Makefile        |  1 +
> >  drivers/clk/samsung/clk-exynos850.c | 63 +++++++++++++++++++++++++++++
> >  2 files changed, 64 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..3192ec9bb90b
> > --- /dev/null
> > +++ b/drivers/clk/samsung/clk-exynos850.c
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2019 Samsung Electronics Co., Ltd.
> > + * Copyright (C) 2021 Linaro Ltd.
> > + *
> > + * Common Clock Framework support for Exynos850 SoC.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clkdev.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
>
> + blank line?
>
> > +#include <dt-bindings/clock/exynos850.h>
> > +
> > +#include "clk.h"
> > +
> > +/* Fixed rate clocks generated outside the SoC */
> > +static struct samsung_fixed_rate_clock exynos850_fixed_rate_ext_clks[] __initdata = {
> > +       FRATE(OSCCLK, "fin_pll", NULL, 0, 26000000),
> > +};
> > +
> > +/*
> > + * Model the UART clock as a fixed-rate clock for now, to make serial driver
> > + * work. This clock is already configured in the bootloader.
> > + */
> > +static const struct samsung_fixed_rate_clock exynos850_peri_clks[] __initconst = {
> > +       FRATE(DOUT_UART, "DOUT_UART", NULL, 0, 200000000),
> > +};
> > +
> > +static const struct of_device_id ext_clk_match[] __initconst = {
>
> > +       { .compatible = "samsung,exynos850-oscclk", .data = (void *)0 },
>
> 0 is the default for static variables.
>
> > +       {},
>
> No comma needed for the terminator lines.
>
> > +};
> > +
> > +void __init exynos850_clk_init(struct device_node *np)
> > +{
> > +       void __iomem *reg_base;
> > +       struct samsung_clk_provider *ctx;
>
> > +       if (!np)
> > +               panic("%s: unable to determine soc\n", __func__);
>
> Check for the sake of additional code?
>
> > +       reg_base = of_iomap(np, 0);
>
> This will fail when np == NULL.
>

Thanks for the review! All your comments will be addressed in v2.

> > +       if (!reg_base)
> > +               panic("%s: failed to map registers\n", __func__);
> > +
> > +       ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
> > +       if (!ctx)
> > +               panic("%s: unable to allocate ctx\n", __func__);
> > +
> > +       samsung_clk_of_register_fixed_ext(ctx,
> > +                       exynos850_fixed_rate_ext_clks,
> > +                       ARRAY_SIZE(exynos850_fixed_rate_ext_clks),
> > +                       ext_clk_match);
> > +
> > +       samsung_clk_register_fixed_rate(ctx, exynos850_peri_clks,
> > +                       ARRAY_SIZE(exynos850_peri_clks));
> > +
> > +       samsung_clk_of_add_provider(np, ctx);
> > +}
> > +
> > +CLK_OF_DECLARE(exynos850_clk, "samsung,exynos850-clock", exynos850_clk_init);
>
>
> --
> With Best Regards,
> Andy Shevchenko