Re: [PATCH v14 3/4] clk: eswin: Add eic7700 clock driver

[Brian Masney <bmasney@redhat.com>]

On Thu, Feb 26, 2026 at 05:06:46PM +0800, dongxuyang@eswincomputing.com wrote:
> From: Xuyang Dong <dongxuyang@eswincomputing.com>
> 
> Add clock drivers for the EIC7700 SoC. The clock controller on the ESWIN
> EIC7700 provides various clocks to different IP blocks within the SoC.
> 
> Signed-off-by: Yifeng Huang <huangyifeng@eswincomputing.com>
> Tested-by: Marcel Ziswiler <marcel@ziswiler.com> # ebc77
> Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>

[snip]

> +static struct platform_driver eic7700_clock_driver = {
> +	.probe	= eic7700_clk_probe,
> +	.driver = {
> +		.name	= "eic7700-clock",
> +		.of_match_table	= eic7700_clock_dt_ids,
> +	},
> +};
> +module_platform_driver(eic7700_clock_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Yifeng Huang<huangyifeng@eswincomputing.com>");
> +MODULE_AUTHOR("Xuyang Dong<dongxuyang@eswincomputing.com>");

Add space between the name and <.

> +MODULE_DESCRIPTION("ESWIN EIC7700 clock controller driver");
> diff --git a/drivers/clk/eswin/clk.c b/drivers/clk/eswin/clk.c
> new file mode 100644
> index 000000000000..3d40d1d7e696
> --- /dev/null
> +++ b/drivers/clk/eswin/clk.c
> @@ -0,0 +1,588 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2026, Beijing ESWIN Computing Technology Co., Ltd..
> + * All rights reserved.
> + *
> + * Authors:
> + *	Yifeng Huang <huangyifeng@eswincomputing.com>
> + *	Xuyang Dong <dongxuyang@eswincomputing.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk-provider.h>
> +#include <linux/iopoll.h>
> +#include <linux/math.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include "common.h"
> +
> +#define PLL_EN_MASK		GENMASK(1, 0)
> +#define PLL_REFDIV_MASK		GENMASK(17, 12)
> +#define PLL_FBDIV_MASK		GENMASK(31, 20)
> +#define PLL_FRAC_MASK		GENMASK(27, 4)
> +#define PLL_POSTDIV1_MASK	GENMASK(10, 8)
> +#define PLL_POSTDIV2_MASK	GENMASK(18, 16)
> +
> +struct eswin_clock_data *eswin_clk_init(struct platform_device *pdev,
> +					size_t nr_clks)
> +{
> +	struct eswin_clock_data *eclk_data;
> +
> +	eclk_data = devm_kzalloc(&pdev->dev,
> +				 struct_size(eclk_data, clk_data.hws, nr_clks),
> +				 GFP_KERNEL);
> +	if (!eclk_data)
> +		return ERR_PTR(-ENOMEM);
> +
> +	eclk_data->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(eclk_data->base))
> +		return ERR_PTR(-EINVAL);
> +
> +	eclk_data->clk_data.num = nr_clks;
> +	/* Avoid returning NULL for unused id */
> +	memset_p((void **)eclk_data->clk_data.hws, ERR_PTR(-ENOENT), nr_clks);

The (void **) cast shouldn't be needed. However, stepping back, this
memory is allocated using kzalloc() above, so that range should already
be zeroed. Unless I am missing something.

With those two minor issues fixed, you can add:

Reviewed-by: Brian Masney <bmasney@redhat.com>