Quoting Tomer Maimon (2024-01-31 10:26:53)
> diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c
> new file mode 100644
> index 000000000000..eacb579d30af
> --- /dev/null
> +++ b/drivers/clk/clk-npcm8xx.c
> @@ -0,0 +1,509 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton NPCM8xx Clock Generator
> + * All the clocks are initialized by the bootloader, so this driver allows only
[...]
> +
> +/* external clock definition */
> +#define NPCM8XX_CLK_S_REFCLK "refclk"
> +
> +/* pll definition */
> +#define NPCM8XX_CLK_S_PLL0 "pll0"
> +#define NPCM8XX_CLK_S_PLL1 "pll1"
> +#define NPCM8XX_CLK_S_PLL2 "pll2"
> +#define NPCM8XX_CLK_S_PLL_GFX "pll_gfx"
> +
> +/* early divider definition */
> +#define NPCM8XX_CLK_S_PLL2_DIV2 "pll2_div2"
> +#define NPCM8XX_CLK_S_PLL_GFX_DIV2 "pll_gfx_div2"
> +#define NPCM8XX_CLK_S_PLL1_DIV2 "pll1_div2"
> +
> +/* mux definition */
> +#define NPCM8XX_CLK_S_CPU_MUX "cpu_mux"
> +
> +/* div definition */
> +#define NPCM8XX_CLK_S_TH "th"
> +#define NPCM8XX_CLK_S_AXI "axi"
Please inline all these string #defines to the place they're used.
> +
> +static struct clk_hw hw_pll1_div2, hw_pll2_div2, hw_gfx_div2, hw_pre_clk;
[..]
> +static struct clk_hw *
> +npcm8xx_clk_register(struct device *dev, const char *name,
> + struct regmap *clk_regmap, unsigned int offset,
> + unsigned long flags, const struct clk_ops *npcm8xx_clk_ops,
> + const struct clk_parent_data *parent_data,
> + const struct clk_hw *parent_hw, u8 num_parents,
> + u8 shift, u32 mask, unsigned long width,
> + const u32 *table, unsigned long clk_flags)
> +{
> + struct npcm8xx_clk *clk;
> + struct clk_init_data init = {};
> + int ret;
> +
> + clk = devm_kzalloc(dev, sizeof(*clk), GFP_KERNEL);
> + if (!clk)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = npcm8xx_clk_ops;
> + init.parent_data = parent_data;
> + init.parent_hws = parent_hw ? &parent_hw : NULL;
Is it necessary to check? Can't it be set unconditionally?
> + init.num_parents = num_parents;
> + init.flags = flags;
> +
> + clk->clk_regmap = clk_regmap;
> + clk->hw.init = &init;
> + clk->offset = offset;
> + clk->shift = shift;
> + clk->mask = mask;
> + clk->width = width;
> + clk->table = table;
> + clk->flags = clk_flags;
> +
> + ret = devm_clk_hw_register(dev, &clk->hw);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return &clk->hw;
[...]
> +
> +static unsigned long npcm8xx_clk_div_get_parent(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct npcm8xx_clk *div = to_npcm8xx_clk(hw);
> + unsigned int val;
> +
> + regmap_read(div->clk_regmap, div->offset, &val);
> + val = val >> div->shift;
> + val &= clk_div_mask(div->width);
> +
> + return divider_recalc_rate(hw, parent_rate, val, NULL, div->flags,
> + div->width);
> +}
> +
> +static const struct clk_ops npcm8xx_clk_div_ops = {
> + .recalc_rate = npcm8xx_clk_div_get_parent,
> +};
> +
> +static int npcm8xx_clk_probe(struct platform_device *pdev)
> +{
> + struct device_node *parent_np = of_get_parent(pdev->dev.of_node);
The parent of this device is not a syscon.
> + struct clk_hw_onecell_data *npcm8xx_clk_data;
> + struct device *dev = &pdev->dev;
> + struct regmap *clk_regmap;
> + struct clk_hw *hw;
> + unsigned int i;
> +
> + npcm8xx_clk_data = devm_kzalloc(dev, struct_size(npcm8xx_clk_data, hws,
> + NPCM8XX_NUM_CLOCKS),
> + GFP_KERNEL);
> + if (!npcm8xx_clk_data)
> + return -ENOMEM;
> +
> + clk_regmap = syscon_node_to_regmap(parent_np);
> + of_node_put(parent_np);
Is there another binding update that is going to move this node to be a
child of the syscon?
gcr: system-controller@f0800000 {
compatible = "nuvoton,npcm845-gcr", "syscon";
reg = <0x0 0xf0800000 0x0 0x1000>;
};
