Quoting Biju Das (2023-03-09 08:55:28)
> diff --git a/drivers/clk/clk-versaclock3.c b/drivers/clk/clk-versaclock3.c
> new file mode 100644
> index 000000000000..6c5c8b37f6af
> --- /dev/null
> +++ b/drivers/clk/clk-versaclock3.c
> @@ -0,0 +1,1139 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Renesas Versaclock 3
> + *
> + * Copyright (C) 2021 Renesas Electronics Corp.
> + */
> +
[...]
> + [vc3_se1] = "se1",
> + [vc3_ref] = "ref"
> +};
> +
> +static const struct clk_parent_data pfdmux_p[] = {
> + { .fw_name = vc3_fin_name, .name = vc3_fin_name },
New drivers should only have .fw_name here. I don't think you're
migrating an existing driver to clk_parent_data so .name should be
removed. And then maybe you'll want to simply use the index instead so
that we don't have to do any string comparisons to find clk parents.
> + { .fw_name = "div2", .name = "div2" }
> +};
> +
[...]
> +
> +static unsigned long vc3_clk_out_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + return parent_rate;
> +}
> +
> +static long vc3_clk_out_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + *prate = clk_hw_round_rate(clk_hw_get_parent(hw), rate);
> +
> + return *prate;
> +}
> +
> +static int vc3_clk_out_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + /*
> + * We must report success. round_rate() propagates rate to the
> + * parent and based on the rate mux changes its parent.
> + */
> +
> + return 0;
> +}
> +
> +const struct clk_ops vc3_clk_out_ops = {
> + .recalc_rate = vc3_clk_out_recalc_rate,
> + .round_rate = vc3_clk_out_round_rate,
> + .set_rate = vc3_clk_out_set_rate,
> +};
Are any of these clk ops necessary? They don't do anything besides pass
up to the parent, so you can set CLK_SET_RATE_PARENT and be done?
> +
> +static bool vc3_regmap_is_writeable(struct device *dev, unsigned int reg)
> +{
> + return true;
> +}
> +
> +static const struct regmap_config vc3_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .cache_type = REGCACHE_RBTREE,
> + .max_register = 0x24,
> + .writeable_reg = vc3_regmap_is_writeable,
> +};
> +
> +static struct clk_hw *vc3_of_clk_get(struct of_phandle_args *clkspec,
> + void *data)
> +{
> + struct vc3_driver_data *vc3 = data;
> + unsigned int idx = clkspec->args[0];
> +
> + if (idx >= ARRAY_SIZE(clk_out_data)) {
> + pr_err("invalid clk index %u for provider %pOF\n", idx, clkspec->np);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return &vc3->clk_out[idx].hw;
> +}
> +
> +static void vc3_divider_type_parse_dt(struct device *dev,
> + struct vc3_driver_data *vc3)
> +{
> + struct device_node *np = dev->of_node;
> + struct property *prop;
> + const __be32 *p;
> + u32 i = 0;
> + u32 val;
> +
> + of_property_for_each_u32(np, "assigned-clock-rates",
This is an interesting use of assigned-clock-rates.
> + prop, p, val) {
> + if (i >= ARRAY_SIZE(div_data))
> + break;
> + if (val)
> + vc3->clk_div[i].flags = CLK_DIVIDER_READ_ONLY;
Why would assigned clock rates change the read only flag?
