On Sat 28 Mar 2020 at 01:32, Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> wrote:
> The SDHC (MMC) controller embeds a clock controller inside one of the
> SDHC registers. The outputs of thisclock controller are dedicated to the
> SDHC controller.
>
> Implement a dedicated clock controller driver so we can keep all the
> clock specific logic outside of the MMC controller driver. There is no
> dedicated clock controller OF node because the hardware is a big SDHC IP
> block with an embedded clock controller (so the .dts doesn't need a
> separate clock controller node). Instead this driver re-uses the regmap
> as registered by the (platform_device) parent.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> ---
> drivers/clk/meson/Kconfig | 9 ++
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/meson-mx-sdhc.c | 212 ++++++++++++++++++++++++++++++
> 3 files changed, 222 insertions(+)
> create mode 100644 drivers/clk/meson/meson-mx-sdhc.c
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index dabeb435d067..8769335d2d46 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -53,6 +53,15 @@ config COMMON_CLK_MESON8B
> S805 (Meson8b) and S812 (Meson8m2) devices. Say Y if you
> want peripherals and CPU frequency scaling to work.
>
> +config COMMON_CLK_MESON_MX_SDHC
> + tristate "Meson MX SDHC MMC Clock Controller Driver"
> + depends on ARCH_MESON
> + select COMMON_CLK_MESON_REGMAP
> + help
> + Support for the SDHC clock controller on Amlogic Meson8/8b/8m2 SoCs
> + devices. Say Y or M if you want to use the SDHC MMC controller.
> + Otherwise say N.
> +
> config COMMON_CLK_GXBB
> bool
> depends on ARCH_MESON
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 6eca2a406ee3..b71c7ae78dbd 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
> +obj-$(CONFIG_COMMON_CLK_MESON_MX_SDHC) += meson-mx-sdhc.o
> diff --git a/drivers/clk/meson/meson-mx-sdhc.c b/drivers/clk/meson/meson-mx-sdhc.c
> new file mode 100644
> index 000000000000..b98a35d99f65
> --- /dev/null
> +++ b/drivers/clk/meson/meson-mx-sdhc.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Amlogic Meson SDHC clock controller
> + *
> + * Copyright (C) 2019 Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> + */
> +
> +#include <dt-bindings/clock/meson-mx-sdhc-clkc.h>
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include "clk-regmap.h"
> +#include "clk-pll.h"
If you need the pll clocks, it should probably appear in the Kconfig
deps as well
> +
> +#define MESON_SDHC_CLKC 0x10
> +
> +static const struct clk_regmap meson_mx_sdhc_src_sel = {
> + .data = &(struct clk_regmap_mux_data){
> + .offset = MESON_SDHC_CLKC,
> + .mask = 0x3,
> + .shift = 16,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_src_sel",
> + .ops = &clk_regmap_mux_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .fw_name = "clkin0", .index = -1, },
> + { .fw_name = "clkin1", .index = -1, },
> + { .fw_name = "clkin2", .index = -1, },
> + { .fw_name = "clkin3", .index = -1, },
When fw_name is specified, setting the index is not necessary
> + },
> + .num_parents = 4,
> + },
> +};
> +
> +static const struct clk_div_table meson_mx_sdhc_div_table[] = {
> + { .div = 6, .val = 5, },
> + { .div = 8, .val = 7, },
> + { .div = 9, .val = 8, },
> + { .div = 10, .val = 9, },
> + { .div = 12, .val = 11, },
> + { .div = 16, .val = 15, },
> + { .div = 18, .val = 17, },
> + { .div = 34, .val = 33, },
> + { .div = 142, .val = 141, },
> + { .div = 850, .val = 849, },
> + { .div = 2126, .val = 2125, },
> + { .div = 4096, .val = 4095, },
> + { /* sentinel */ }
> +};
> +
> +static const struct clk_regmap meson_mx_sdhc_div = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = MESON_SDHC_CLKC,
> + .shift = 0,
> + .width = 12,
> + .table = meson_mx_sdhc_div_table,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_div",
> + .ops = &clk_regmap_divider_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "sdhc_src_sel", .index = -1, },
Any reason for using the lagacy names and not the clk_hw pointers ?
Same comment for the rest of the clocks
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static const struct clk_regmap meson_mx_sdhc_mod_clk_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = MESON_SDHC_CLKC,
> + .bit_idx = 15,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_mod_clk_on",
> + .ops = &clk_regmap_gate_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "sdhc_div", .index = -1, },
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_GATE,
Why can't the clock change rate unless gated ? Maybe you prefer to
change the rate in the mmc while clock is gated, but this is the
handling of the clock by the mmc driver, not a constraint of the actual
clock HW, isn't it ?
Also, this is a gate so I suppose the rate propagates through it ?
Can you explain why CLK_SET_RATE_PARENT is not set ?
> + },
> +};
> +
> +static const struct clk_regmap meson_mx_sdhc_tx_clk_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = MESON_SDHC_CLKC,
> + .bit_idx = 14,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_tx_clk_on",
> + .ops = &clk_regmap_gate_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "sdhc_div", .index = -1, },
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_GATE,
> + },
> +};
> +
> +static const struct clk_regmap meson_mx_sdhc_rx_clk_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = MESON_SDHC_CLKC,
> + .bit_idx = 13,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_rx_clk_on",
> + .ops = &clk_regmap_gate_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "sdhc_div", .index = -1, },
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
Ok so apparently you only want to set the rate through the RX clock.
You are free to call set_rate() only on this clock in the mmc driver.
However, I don't think this should reflect as clock constraints.
> + },
> +};
> +
> +static const struct clk_regmap meson_mx_sdhc_sd_clk_en = {
> + .data = &(struct clk_regmap_gate_data){
> + .offset = MESON_SDHC_CLKC,
> + .bit_idx = 12,
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "sdhc_sd_clk_on",
> + .ops = &clk_regmap_gate_ops,
> + .parent_data = (const struct clk_parent_data[]) {
> + { .name = "sdhc_div", .index = -1, },
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
... now I lost with these flags. I'm sure there is an idea related to
the mmc driver. Clockwise, I don't get it.
> + },
> +};
> +
> +static const struct clk_regmap *meson_mx_sdhc_clk_regmaps[] = {
> + [SDHC_CLKID_SRC_SEL] = &meson_mx_sdhc_src_sel,
> + [SDHC_CLKID_DIV] = &meson_mx_sdhc_div,
> + [SDHC_CLKID_MOD_CLK] = &meson_mx_sdhc_mod_clk_en,
> + [SDHC_CLKID_SD_CLK] = &meson_mx_sdhc_sd_clk_en,
> + [SDHC_CLKID_TX_CLK] = &meson_mx_sdhc_tx_clk_en,
> + [SDHC_CLKID_RX_CLK] = &meson_mx_sdhc_rx_clk_en,
> +};
> +
> +#define MESON_MX_SDHC_NUM_CLKS ARRAY_SIZE(meson_mx_sdhc_clk_regmaps)
> +
> +static int meson_mx_sdhc_clkc_probe(struct platform_device *pdev)
> +{
> + struct device *parent = pdev->dev.parent;
> + struct clk_hw_onecell_data *onecell_data;
> + struct clk_regmap *clk_regmap;
> + struct regmap *regmap;
> + int i, ret;
> +
> + regmap = dev_get_regmap(parent, NULL);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + clk_regmap = devm_kcalloc(parent, sizeof(*clk_regmap),
> + MESON_MX_SDHC_NUM_CLKS, GFP_KERNEL);
> + if (!clk_regmap)
> + return -ENOMEM;
> +
> + onecell_data = devm_kzalloc(parent,
> + struct_size(onecell_data, hws,
> + MESON_MX_SDHC_NUM_CLKS),
> + GFP_KERNEL);
> + if (!onecell_data)
> + return -ENOMEM;
> +
> + for (i = 0; i < MESON_MX_SDHC_NUM_CLKS; i++) {
> + memcpy(&clk_regmap[i], meson_mx_sdhc_clk_regmaps[i],
> + sizeof(*clk_regmap));
> +
> + clk_regmap[i].map = regmap;
> + onecell_data->hws[i] = &clk_regmap[i].hw;
> +
> + ret = devm_clk_hw_register(parent, onecell_data->hws[i]);
> + if (ret) {
> + dev_err(parent,
> + "Registration of SDHC clock %d failed\n", i);
> + return ret;
> + }
> + }
> +
> + onecell_data->num = MESON_MX_SDHC_NUM_CLKS;
> +
> + return devm_of_clk_add_hw_provider(parent, of_clk_hw_onecell_get,
> + onecell_data);
> +}
> +
> +static const struct platform_device_id meson_mx_sdhc_clkc_ids[] = {
> + { "meson8-sdhc-clkc" },
> + { "meson8b-sdhc-clkc" },
> + { "meson8m2-sdhc-clkc" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, meson_mx_sdhc_clkc_ids);
> +
> +static struct platform_driver meson_mx_sdhc_clkc_driver = {
> + .id_table = meson_mx_sdhc_clkc_ids,
> + .probe = meson_mx_sdhc_clkc_probe,
> + .driver = {
> + .name = "meson-mx-sdhc-clkc",
> + },
> +};
> +module_platform_driver(meson_mx_sdhc_clkc_driver);
> +
> +MODULE_DESCRIPTION("Amlogic Meson8/8b/8m2 SDHC clock controller driver");
> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
