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

From: qinjian[覃健]
Date: Tue Jun 14 2022 - 22:33:49 EST


> > 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?

Yes, I copy/paste from DT.
Did I need changed it to 'GPL-2.0-only'?

>
> > +/*
> > + * 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-provider.h:
struct clk_hw *__devm_clk_hw_register_gate(struct device *dev,
struct device_node *np, const char *name,
const char *parent_name, const struct clk_hw *parent_hw,
const struct clk_parent_data *parent_data,
unsigned long flags,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock);
#define devm_clk_hw_register_gate(dev, name, parent_name, flags, reg, bit_idx,\
clk_gate_flags, lock) \
__devm_clk_hw_register_gate((dev), NULL, (name), (parent_name), NULL, \
NULL, (flags), (reg), (bit_idx), \
(clk_gate_flags), (lock))

Should I use __devm_clk_hw_register_gate() with parent_data
or
devm_clk_hw_register_gate() with parent_name?