Re: [PATCH v18 05/10] clk: Add Sunplus SP7021 clock driver

[Stephen Boyd <sboyd@kernel.org>]

Quoting Qin Jian (2022-06-14 01:31:04)
> diff --git a/drivers/clk/clk-sp7021.c b/drivers/clk/clk-sp7021.c
> new file mode 100644
> index 000000000..0caef1bc7
> --- /dev/null
> +++ b/drivers/clk/clk-sp7021.c
> @@ -0,0 +1,725 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

This isn't a common license on driver files. Is it intended? Or
copy/paste from DT?

> +/*
> + * Copyright (C) Sunplus Technology Co., Ltd.
> + *       All rights reserved.
> + */
[...]
> +
> +static int sp7021_clk_probe(struct platform_device *pdev)
> +{
[...]
> +
> +       /* gates */
> +       for (i = 0; i < ARRAY_SIZE(sp_clk_gates); i++) {
> +               char name[10];
> +               u32 j = sp_clk_gates[i].reg;
> +               struct clk_parent_data *pd = sp_clk_gates[i].ext_parent ? &pd_ext : &pd_sys;
> +
> +               sprintf(name, "%02d_0x%02x", i, j);
> +               hws[i] = clk_hw_register_gate_parent_data(dev, name, pd, 0,

This needs to be devm as well. Otherwise if the driver is unbound we'll
leak these registered clks.

> +                                                         clk_base + (j >> 4) * 4,
> +                                                         j & 0x0f,
> +                                                         CLK_GATE_HIWORD_MASK,
> +                                                         NULL);
> +               if (IS_ERR(hws[i])) {
> +                       ret = PTR_ERR(hws[i]);
> +                       goto unregister_gates;
> +               }
> +       }
> +
> +       clk_data->num = CLK_MAX;
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> +       if (ret)
> +               goto unregister_gates;
> +
> +       return 0;
> +
> +unregister_gates:
> +       while (i--)
> +               clk_hw_unregister_gate(hws[i]);
> +
> +       return ret;
> +}
> +