diff mbox series

[03/18] clk: meson: migrate a1 clock drivers out of hw_onecell_data to drop NR_CLKS

Message ID 20230607-topic-amlogic-upstream-clkid-public-migration-v1-3-9676afa6b22c@linaro.org (mailing list archive)
State Superseded
Delegated to: Neil Armstrong
Headers show
Series clk: meson: move all private clk IDs to public dt-bindings headers | expand

Commit Message

Neil Armstrong June 7, 2023, 10:56 a.m. UTC
The way hw_onecell_data is declared:
struct clk_hw_onecell_data {
	unsigned int num;
	struct clk_hw *hws[];
};

makes it impossible to have the clk_hw table declared outside while
using ARRAY_SIZE() to determine ".num" due to ".hws" being a flexible
array member.

Completely move out of hw_onecell_data and add a custom
devm_of_clk_add_hw_provider() "get" callback to retrieve the clk_hw
in order to finally get rid on the NR_CLKS define.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/clk/meson/a1-peripherals.c | 343 +++++++++++++++++++------------------
 drivers/clk/meson/a1-peripherals.h |   1 -
 drivers/clk/meson/a1-pll.c         |  57 ++++--
 drivers/clk/meson/a1-pll.h         |   1 -
 4 files changed, 219 insertions(+), 183 deletions(-)

Comments

Jerome Brunet June 8, 2023, 12:45 p.m. UTC | #1
>  
> +struct meson_a1_pll_clks {
> +	struct clk_hw **hw_clks;
> +	unsigned int hw_clk_num;
> +};
> +
> +static struct meson_a1_pll_clks a1_pll_clks = {
> +	.hw_clks = a1_pll_hw_clks,
> +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
> +};
> +
> +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
> +{
> +	const struct meson_a1_pll_clks *data = clk_data;
> +	unsigned int idx = clkspec->args[0];
> +
> +	if (idx >= data->hw_clk_num) {
> +		pr_err("%s: invalid index %u\n", __func__, idx);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return data->hw_clks[idx];
> +}

I'd prefer to have a single struct type and and single custom
callback for the different SoC please.
Neil Armstrong June 8, 2023, 12:53 p.m. UTC | #2
On 08/06/2023 14:45, Jerome Brunet wrote:
>>   
>> +struct meson_a1_pll_clks {
>> +	struct clk_hw **hw_clks;
>> +	unsigned int hw_clk_num;
>> +};
>> +
>> +static struct meson_a1_pll_clks a1_pll_clks = {
>> +	.hw_clks = a1_pll_hw_clks,
>> +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
>> +};
>> +
>> +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
>> +{
>> +	const struct meson_a1_pll_clks *data = clk_data;
>> +	unsigned int idx = clkspec->args[0];
>> +
>> +	if (idx >= data->hw_clk_num) {
>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return data->hw_clks[idx];
>> +}
> 
> I'd prefer to have a single struct type and and single custom
> callback for the different SoC please.

Sure, I've written a common code for that, but I have a hard time finding
a proper naming for it... so I choosed meson-clkc since it could have
more common helper code for duplicated code over the clk driver:

===================================><============================================================================
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 8ce846fdbe43..9070dcfd9e71 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
  	tristate
  	select COMMON_CLK_MESON_REGMAP

+config COMMON_CLK_MESON_CLKC
+	tristate
+
  config COMMON_CLK_MESON_AO_CLKC
  	tristate
  	select COMMON_CLK_MESON_REGMAP
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index d5288662881d..13c6db466986 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -1,6 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0-only
  # Amlogic clock drivers

+obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
  obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
  obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
  obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
new file mode 100644
index 000000000000..fa98b9d09011
--- /dev/null
+++ b/drivers/clk/meson/meson-clkc.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
+ */
+
+#include <linux/of_device.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include "meson-clkc.h"
+
+struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
+{
+	const struct meson_clk_hw_data *data = clk_hw_data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx >= data->num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return data->hws[idx];
+}
+EXPORT_SYMBOL_GPL(meson_clk_hw_get);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
new file mode 100644
index 000000000000..e3bad2aa17eb
--- /dev/null
+++ b/drivers/clk/meson/meson-clkc.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
+ */
+
+#ifndef __MESON_HW_CLKC_H__
+#define __MESON_HW_CLKC_H__
+
+#include <linux/of_device.h>
+#include <linux/clk-provider.h>
+
+struct meson_clk_hw_data {
+	struct clk_hw	**hws;
+	unsigned int	num;
+};
+
+struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
+
+#endif
===================================><============================================================================

If it's ok I'll send a v2 using this.

Thanks,
Neil
Dmitry Rokosov June 9, 2023, 11:48 a.m. UTC | #3
Hello Neil,

On Thu, Jun 08, 2023 at 02:53:50PM +0200, Neil Armstrong wrote:
> On 08/06/2023 14:45, Jerome Brunet wrote:
> > > +struct meson_a1_pll_clks {
> > > +	struct clk_hw **hw_clks;
> > > +	unsigned int hw_clk_num;
> > > +};
> > > +
> > > +static struct meson_a1_pll_clks a1_pll_clks = {
> > > +	.hw_clks = a1_pll_hw_clks,
> > > +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
> > > +};
> > > +
> > > +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
> > > +{
> > > +	const struct meson_a1_pll_clks *data = clk_data;
> > > +	unsigned int idx = clkspec->args[0];
> > > +
> > > +	if (idx >= data->hw_clk_num) {
> > > +		pr_err("%s: invalid index %u\n", __func__, idx);
> > > +		return ERR_PTR(-EINVAL);
> > > +	}
> > > +
> > > +	return data->hw_clks[idx];
> > > +}
> > 
> > I'd prefer to have a single struct type and and single custom
> > callback for the different SoC please.
> 
> Sure, I've written a common code for that, but I have a hard time finding
> a proper naming for it... so I choosed meson-clkc since it could have
> more common helper code for duplicated code over the clk driver:
> 
> ===================================><============================================================================
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 8ce846fdbe43..9070dcfd9e71 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
> 
> +config COMMON_CLK_MESON_CLKC
> +	tristate
> +
>  config COMMON_CLK_MESON_AO_CLKC
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index d5288662881d..13c6db466986 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  # Amlogic clock drivers
> 
> +obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
>  obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
>  obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
> diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
> new file mode 100644
> index 000000000000..fa98b9d09011
> --- /dev/null
> +++ b/drivers/clk/meson/meson-clkc.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> + */
> +
> +#include <linux/of_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include "meson-clkc.h"
> +
> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
> +{
> +	const struct meson_clk_hw_data *data = clk_hw_data;
> +	unsigned int idx = clkspec->args[0];
> +
> +	if (idx >= data->num) {
> +		pr_err("%s: invalid index %u\n", __func__, idx);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return data->hws[idx];
> +}
> +EXPORT_SYMBOL_GPL(meson_clk_hw_get);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
> new file mode 100644
> index 000000000000..e3bad2aa17eb
> --- /dev/null
> +++ b/drivers/clk/meson/meson-clkc.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> + */
> +
> +#ifndef __MESON_HW_CLKC_H__
> +#define __MESON_HW_CLKC_H__
> +
> +#include <linux/of_device.h>
> +#include <linux/clk-provider.h>
> +
> +struct meson_clk_hw_data {
> +	struct clk_hw	**hws;
> +	unsigned int	num;
> +};
> +
> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
> +
> +#endif
> ===================================><============================================================================
> 
> If it's ok I'll send a v2 using this.
> 
> Thanks,
> Neil

In addition, I propose consolidating the probe() routine of the a1
clocks into a common part, which can be utilized for s4 and other
similar clocks. This solution was presented in the early a1 review
stages of this patch set.

https://lore.kernel.org/linux-amlogic/20221201225703.6507-7-ddrokosov@sberdevices.ru/

Maybe, it can be generalized for the all meson clock controller drivers.
Neil Armstrong June 9, 2023, 12:47 p.m. UTC | #4
Hi,

On 09/06/2023 13:48, Dmitry Rokosov wrote:
> Hello Neil,
> 
> On Thu, Jun 08, 2023 at 02:53:50PM +0200, Neil Armstrong wrote:
>> On 08/06/2023 14:45, Jerome Brunet wrote:
>>>> +struct meson_a1_pll_clks {
>>>> +	struct clk_hw **hw_clks;
>>>> +	unsigned int hw_clk_num;
>>>> +};
>>>> +
>>>> +static struct meson_a1_pll_clks a1_pll_clks = {
>>>> +	.hw_clks = a1_pll_hw_clks,
>>>> +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
>>>> +};
>>>> +
>>>> +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
>>>> +{
>>>> +	const struct meson_a1_pll_clks *data = clk_data;
>>>> +	unsigned int idx = clkspec->args[0];
>>>> +
>>>> +	if (idx >= data->hw_clk_num) {
>>>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>>>> +		return ERR_PTR(-EINVAL);
>>>> +	}
>>>> +
>>>> +	return data->hw_clks[idx];
>>>> +}
>>>
>>> I'd prefer to have a single struct type and and single custom
>>> callback for the different SoC please.
>>
>> Sure, I've written a common code for that, but I have a hard time finding
>> a proper naming for it... so I choosed meson-clkc since it could have
>> more common helper code for duplicated code over the clk driver:
>>
>> ===================================><============================================================================
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 8ce846fdbe43..9070dcfd9e71 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>   	tristate
>>   	select COMMON_CLK_MESON_REGMAP
>>
>> +config COMMON_CLK_MESON_CLKC
>> +	tristate
>> +
>>   config COMMON_CLK_MESON_AO_CLKC
>>   	tristate
>>   	select COMMON_CLK_MESON_REGMAP
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index d5288662881d..13c6db466986 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   # Amlogic clock drivers
>>
>> +obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
>> diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
>> new file mode 100644
>> index 000000000000..fa98b9d09011
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-clkc.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
>> + */
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include "meson-clkc.h"
>> +
>> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
>> +{
>> +	const struct meson_clk_hw_data *data = clk_hw_data;
>> +	unsigned int idx = clkspec->args[0];
>> +
>> +	if (idx >= data->num) {
>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return data->hws[idx];
>> +}
>> +EXPORT_SYMBOL_GPL(meson_clk_hw_get);
>> +
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
>> new file mode 100644
>> index 000000000000..e3bad2aa17eb
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-clkc.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
>> + */
>> +
>> +#ifndef __MESON_HW_CLKC_H__
>> +#define __MESON_HW_CLKC_H__
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +
>> +struct meson_clk_hw_data {
>> +	struct clk_hw	**hws;
>> +	unsigned int	num;
>> +};
>> +
>> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
>> +
>> +#endif
>> ===================================><============================================================================
>>
>> If it's ok I'll send a v2 using this.
>>
>> Thanks,
>> Neil
> 
> In addition, I propose consolidating the probe() routine of the a1
> clocks into a common part, which can be utilized for s4 and other
> similar clocks. This solution was presented in the early a1 review
> stages of this patch set.
> 
> https://lore.kernel.org/linux-amlogic/20221201225703.6507-7-ddrokosov@sberdevices.ru/
> 
> Maybe, it can be generalized for the all meson clock controller drivers.
> 

Sure, the goal of that is to get rid of NR_CLKS & private IDS only,
then yes afterwards any common clkc function will be welcome !

Neil
Dmitry Rokosov June 9, 2023, 12:57 p.m. UTC | #5
On Fri, Jun 09, 2023 at 02:47:04PM +0200, Neil Armstrong wrote:
> Hi,
> 
> On 09/06/2023 13:48, Dmitry Rokosov wrote:
> > Hello Neil,
> > 
> > On Thu, Jun 08, 2023 at 02:53:50PM +0200, Neil Armstrong wrote:
> > > On 08/06/2023 14:45, Jerome Brunet wrote:
> > > > > +struct meson_a1_pll_clks {
> > > > > +	struct clk_hw **hw_clks;
> > > > > +	unsigned int hw_clk_num;
> > > > > +};
> > > > > +
> > > > > +static struct meson_a1_pll_clks a1_pll_clks = {
> > > > > +	.hw_clks = a1_pll_hw_clks,
> > > > > +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
> > > > > +};
> > > > > +
> > > > > +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
> > > > > +{
> > > > > +	const struct meson_a1_pll_clks *data = clk_data;
> > > > > +	unsigned int idx = clkspec->args[0];
> > > > > +
> > > > > +	if (idx >= data->hw_clk_num) {
> > > > > +		pr_err("%s: invalid index %u\n", __func__, idx);
> > > > > +		return ERR_PTR(-EINVAL);
> > > > > +	}
> > > > > +
> > > > > +	return data->hw_clks[idx];
> > > > > +}
> > > > 
> > > > I'd prefer to have a single struct type and and single custom
> > > > callback for the different SoC please.
> > > 
> > > Sure, I've written a common code for that, but I have a hard time finding
> > > a proper naming for it... so I choosed meson-clkc since it could have
> > > more common helper code for duplicated code over the clk driver:
> > > 
> > > ===================================><============================================================================
> > > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> > > index 8ce846fdbe43..9070dcfd9e71 100644
> > > --- a/drivers/clk/meson/Kconfig
> > > +++ b/drivers/clk/meson/Kconfig
> > > @@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
> > >   	tristate
> > >   	select COMMON_CLK_MESON_REGMAP
> > > 
> > > +config COMMON_CLK_MESON_CLKC
> > > +	tristate
> > > +
> > >   config COMMON_CLK_MESON_AO_CLKC
> > >   	tristate
> > >   	select COMMON_CLK_MESON_REGMAP
> > > diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> > > index d5288662881d..13c6db466986 100644
> > > --- a/drivers/clk/meson/Makefile
> > > +++ b/drivers/clk/meson/Makefile
> > > @@ -1,6 +1,7 @@
> > >   # SPDX-License-Identifier: GPL-2.0-only
> > >   # Amlogic clock drivers
> > > 
> > > +obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
> > >   obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
> > >   obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
> > >   obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
> > > diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
> > > new file mode 100644
> > > index 000000000000..fa98b9d09011
> > > --- /dev/null
> > > +++ b/drivers/clk/meson/meson-clkc.c
> > > @@ -0,0 +1,25 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> > > + */
> > > +
> > > +#include <linux/of_device.h>
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/module.h>
> > > +#include "meson-clkc.h"
> > > +
> > > +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
> > > +{
> > > +	const struct meson_clk_hw_data *data = clk_hw_data;
> > > +	unsigned int idx = clkspec->args[0];
> > > +
> > > +	if (idx >= data->num) {
> > > +		pr_err("%s: invalid index %u\n", __func__, idx);
> > > +		return ERR_PTR(-EINVAL);
> > > +	}
> > > +
> > > +	return data->hws[idx];
> > > +}
> > > +EXPORT_SYMBOL_GPL(meson_clk_hw_get);
> > > +
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
> > > new file mode 100644
> > > index 000000000000..e3bad2aa17eb
> > > --- /dev/null
> > > +++ b/drivers/clk/meson/meson-clkc.h
> > > @@ -0,0 +1,19 @@
> > > +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> > > +/*
> > > + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> > > + */
> > > +
> > > +#ifndef __MESON_HW_CLKC_H__
> > > +#define __MESON_HW_CLKC_H__
> > > +
> > > +#include <linux/of_device.h>
> > > +#include <linux/clk-provider.h>
> > > +
> > > +struct meson_clk_hw_data {
> > > +	struct clk_hw	**hws;
> > > +	unsigned int	num;
> > > +};
> > > +
> > > +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
> > > +
> > > +#endif
> > > ===================================><============================================================================
> > > 
> > > If it's ok I'll send a v2 using this.
> > > 
> > > Thanks,
> > > Neil
> > 
> > In addition, I propose consolidating the probe() routine of the a1
> > clocks into a common part, which can be utilized for s4 and other
> > similar clocks. This solution was presented in the early a1 review
> > stages of this patch set.
> > 
> > https://lore.kernel.org/linux-amlogic/20221201225703.6507-7-ddrokosov@sberdevices.ru/
> > 
> > Maybe, it can be generalized for the all meson clock controller drivers.
> > 
> 
> Sure, the goal of that is to get rid of NR_CLKS & private IDS only,
> then yes afterwards any common clkc function will be welcome !
> 
> Neil
> 

Understood, I fully endorse and support this approach!
Jerome Brunet June 9, 2023, 2:30 p.m. UTC | #6
On Thu 08 Jun 2023 at 14:53, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 08/06/2023 14:45, Jerome Brunet wrote:
>>>   +struct meson_a1_pll_clks {
>>> +	struct clk_hw **hw_clks;
>>> +	unsigned int hw_clk_num;
>>> +};
>>> +
>>> +static struct meson_a1_pll_clks a1_pll_clks = {
>>> +	.hw_clks = a1_pll_hw_clks,
>>> +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
>>> +};
>>> +
>>> +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
>>> +{
>>> +	const struct meson_a1_pll_clks *data = clk_data;
>>> +	unsigned int idx = clkspec->args[0];
>>> +
>>> +	if (idx >= data->hw_clk_num) {
>>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>>> +		return ERR_PTR(-EINVAL);
>>> +	}
>>> +
>>> +	return data->hw_clks[idx];
>>> +}
>> I'd prefer to have a single struct type and and single custom
>> callback for the different SoC please.
>
> Sure, I've written a common code for that, but I have a hard time finding
> a proper naming for it... so I choosed meson-clkc since it could have
> more common helper code for duplicated code over the clk driver:

Agreed. meson-clkc-utils maybe ?

>
> ===================================><============================================================================
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 8ce846fdbe43..9070dcfd9e71 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
>
> +config COMMON_CLK_MESON_CLKC
> +	tristate
> +
>  config COMMON_CLK_MESON_AO_CLKC
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index d5288662881d..13c6db466986 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  # Amlogic clock drivers
>
> +obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
>  obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
>  obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
>  obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
> diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
> new file mode 100644
> index 000000000000..fa98b9d09011
> --- /dev/null
> +++ b/drivers/clk/meson/meson-clkc.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> + */
> +
> +#include <linux/of_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include "meson-clkc.h"
> +
> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
> +{
> +	const struct meson_clk_hw_data *data = clk_hw_data;
> +	unsigned int idx = clkspec->args[0];
> +
> +	if (idx >= data->num) {
> +		pr_err("%s: invalid index %u\n", __func__, idx);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return data->hws[idx];
> +}
> +EXPORT_SYMBOL_GPL(meson_clk_hw_get);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
> new file mode 100644
> index 000000000000..e3bad2aa17eb
> --- /dev/null
> +++ b/drivers/clk/meson/meson-clkc.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
> + */
> +
> +#ifndef __MESON_HW_CLKC_H__
> +#define __MESON_HW_CLKC_H__
> +
> +#include <linux/of_device.h>
> +#include <linux/clk-provider.h>
> +
> +struct meson_clk_hw_data {
> +	struct clk_hw	**hws;
> +	unsigned int	num;
> +};
> +
> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
> +
> +#endif
> ===================================><============================================================================
>
> If it's ok I'll send a v2 using this.
>
> Thanks,
> Neil
Neil Armstrong June 9, 2023, 3:32 p.m. UTC | #7
On 09/06/2023 16:30, Jerome Brunet wrote:
> 
> On Thu 08 Jun 2023 at 14:53, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
>> On 08/06/2023 14:45, Jerome Brunet wrote:
>>>>    +struct meson_a1_pll_clks {
>>>> +	struct clk_hw **hw_clks;
>>>> +	unsigned int hw_clk_num;
>>>> +};
>>>> +
>>>> +static struct meson_a1_pll_clks a1_pll_clks = {
>>>> +	.hw_clks = a1_pll_hw_clks,
>>>> +	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
>>>> +};
>>>> +
>>>> +static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
>>>> +{
>>>> +	const struct meson_a1_pll_clks *data = clk_data;
>>>> +	unsigned int idx = clkspec->args[0];
>>>> +
>>>> +	if (idx >= data->hw_clk_num) {
>>>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>>>> +		return ERR_PTR(-EINVAL);
>>>> +	}
>>>> +
>>>> +	return data->hw_clks[idx];
>>>> +}
>>> I'd prefer to have a single struct type and and single custom
>>> callback for the different SoC please.
>>
>> Sure, I've written a common code for that, but I have a hard time finding
>> a proper naming for it... so I choosed meson-clkc since it could have
>> more common helper code for duplicated code over the clk driver:
> 
> Agreed. meson-clkc-utils maybe ?

Ack seems better

Thanks
Neil

> 
>>
>> ===================================><============================================================================
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 8ce846fdbe43..9070dcfd9e71 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -30,6 +30,9 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>   	tristate
>>   	select COMMON_CLK_MESON_REGMAP
>>
>> +config COMMON_CLK_MESON_CLKC
>> +	tristate
>> +
>>   config COMMON_CLK_MESON_AO_CLKC
>>   	tristate
>>   	select COMMON_CLK_MESON_REGMAP
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index d5288662881d..13c6db466986 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -1,6 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0-only
>>   # Amlogic clock drivers
>>
>> +obj-$(CONFIG_COMMON_CLK_MESON_CLKC) += meson-clkc.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_AO_CLKC) += meson-aoclk.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_CPU_DYNDIV) += clk-cpu-dyndiv.o
>>   obj-$(CONFIG_COMMON_CLK_MESON_DUALDIV) += clk-dualdiv.o
>> diff --git a/drivers/clk/meson/meson-clkc.c b/drivers/clk/meson/meson-clkc.c
>> new file mode 100644
>> index 000000000000..fa98b9d09011
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-clkc.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
>> + */
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include "meson-clkc.h"
>> +
>> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data)
>> +{
>> +	const struct meson_clk_hw_data *data = clk_hw_data;
>> +	unsigned int idx = clkspec->args[0];
>> +
>> +	if (idx >= data->num) {
>> +		pr_err("%s: invalid index %u\n", __func__, idx);
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	return data->hws[idx];
>> +}
>> +EXPORT_SYMBOL_GPL(meson_clk_hw_get);
>> +
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/clk/meson/meson-clkc.h b/drivers/clk/meson/meson-clkc.h
>> new file mode 100644
>> index 000000000000..e3bad2aa17eb
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-clkc.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2023 Neil Armstrong <neil.armstrong@linaro.org>
>> + */
>> +
>> +#ifndef __MESON_HW_CLKC_H__
>> +#define __MESON_HW_CLKC_H__
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +
>> +struct meson_clk_hw_data {
>> +	struct clk_hw	**hws;
>> +	unsigned int	num;
>> +};
>> +
>> +struct clk_hw *meson_clk_hw_get(struct of_phandle_args *clkspec, void *clk_hw_data);
>> +
>> +#endif
>> ===================================><============================================================================
>>
>> If it's ok I'll send a v2 using this.
>>
>> Thanks,
>> Neil
>
diff mbox series

Patch

diff --git a/drivers/clk/meson/a1-peripherals.c b/drivers/clk/meson/a1-peripherals.c
index b320134fefeb..094246cb5f6c 100644
--- a/drivers/clk/meson/a1-peripherals.c
+++ b/drivers/clk/meson/a1-peripherals.c
@@ -1866,165 +1866,161 @@  static MESON_GATE(rom,		AXI_CLK_EN,	11);
 static MESON_GATE(prod_i2c,	AXI_CLK_EN,	12);
 
 /* Array of all clocks registered by this provider */
-static struct clk_hw_onecell_data a1_periphs_clks = {
-	.hws = {
-		[CLKID_XTAL_IN]			= &xtal_in.hw,
-		[CLKID_FIXPLL_IN]		= &fixpll_in.hw,
-		[CLKID_USB_PHY_IN]		= &usb_phy_in.hw,
-		[CLKID_USB_CTRL_IN]		= &usb_ctrl_in.hw,
-		[CLKID_HIFIPLL_IN]		= &hifipll_in.hw,
-		[CLKID_SYSPLL_IN]		= &syspll_in.hw,
-		[CLKID_DDS_IN]			= &dds_in.hw,
-		[CLKID_SYS]			= &sys.hw,
-		[CLKID_CLKTREE]			= &clktree.hw,
-		[CLKID_RESET_CTRL]		= &reset_ctrl.hw,
-		[CLKID_ANALOG_CTRL]		= &analog_ctrl.hw,
-		[CLKID_PWR_CTRL]		= &pwr_ctrl.hw,
-		[CLKID_PAD_CTRL]		= &pad_ctrl.hw,
-		[CLKID_SYS_CTRL]		= &sys_ctrl.hw,
-		[CLKID_TEMP_SENSOR]		= &temp_sensor.hw,
-		[CLKID_AM2AXI_DIV]		= &am2axi_dev.hw,
-		[CLKID_SPICC_B]			= &spicc_b.hw,
-		[CLKID_SPICC_A]			= &spicc_a.hw,
-		[CLKID_MSR]			= &msr.hw,
-		[CLKID_AUDIO]			= &audio.hw,
-		[CLKID_JTAG_CTRL]		= &jtag_ctrl.hw,
-		[CLKID_SARADC_EN]		= &saradc_en.hw,
-		[CLKID_PWM_EF]			= &pwm_ef.hw,
-		[CLKID_PWM_CD]			= &pwm_cd.hw,
-		[CLKID_PWM_AB]			= &pwm_ab.hw,
-		[CLKID_CEC]			= &cec.hw,
-		[CLKID_I2C_S]			= &i2c_s.hw,
-		[CLKID_IR_CTRL]			= &ir_ctrl.hw,
-		[CLKID_I2C_M_D]			= &i2c_m_d.hw,
-		[CLKID_I2C_M_C]			= &i2c_m_c.hw,
-		[CLKID_I2C_M_B]			= &i2c_m_b.hw,
-		[CLKID_I2C_M_A]			= &i2c_m_a.hw,
-		[CLKID_ACODEC]			= &acodec.hw,
-		[CLKID_OTP]			= &otp.hw,
-		[CLKID_SD_EMMC_A]		= &sd_emmc_a.hw,
-		[CLKID_USB_PHY]			= &usb_phy.hw,
-		[CLKID_USB_CTRL]		= &usb_ctrl.hw,
-		[CLKID_SYS_DSPB]		= &sys_dspb.hw,
-		[CLKID_SYS_DSPA]		= &sys_dspa.hw,
-		[CLKID_DMA]			= &dma.hw,
-		[CLKID_IRQ_CTRL]		= &irq_ctrl.hw,
-		[CLKID_NIC]			= &nic.hw,
-		[CLKID_GIC]			= &gic.hw,
-		[CLKID_UART_C]			= &uart_c.hw,
-		[CLKID_UART_B]			= &uart_b.hw,
-		[CLKID_UART_A]			= &uart_a.hw,
-		[CLKID_SYS_PSRAM]		= &sys_psram.hw,
-		[CLKID_RSA]			= &rsa.hw,
-		[CLKID_CORESIGHT]		= &coresight.hw,
-		[CLKID_AM2AXI_VAD]		= &am2axi_vad.hw,
-		[CLKID_AUDIO_VAD]		= &audio_vad.hw,
-		[CLKID_AXI_DMC]			= &axi_dmc.hw,
-		[CLKID_AXI_PSRAM]		= &axi_psram.hw,
-		[CLKID_RAMB]			= &ramb.hw,
-		[CLKID_RAMA]			= &rama.hw,
-		[CLKID_AXI_SPIFC]		= &axi_spifc.hw,
-		[CLKID_AXI_NIC]			= &axi_nic.hw,
-		[CLKID_AXI_DMA]			= &axi_dma.hw,
-		[CLKID_CPU_CTRL]		= &cpu_ctrl.hw,
-		[CLKID_ROM]			= &rom.hw,
-		[CLKID_PROC_I2C]		= &prod_i2c.hw,
-		[CLKID_DSPA_SEL]		= &dspa_sel.hw,
-		[CLKID_DSPB_SEL]		= &dspb_sel.hw,
-		[CLKID_DSPA_EN]			= &dspa_en.hw,
-		[CLKID_DSPA_EN_NIC]		= &dspa_en_nic.hw,
-		[CLKID_DSPB_EN]			= &dspb_en.hw,
-		[CLKID_DSPB_EN_NIC]		= &dspb_en_nic.hw,
-		[CLKID_RTC]			= &rtc.hw,
-		[CLKID_CECA_32K]		= &ceca_32k_out.hw,
-		[CLKID_CECB_32K]		= &cecb_32k_out.hw,
-		[CLKID_24M]			= &clk_24m.hw,
-		[CLKID_12M]			= &clk_12m.hw,
-		[CLKID_FCLK_DIV2_DIVN]		= &fclk_div2_divn.hw,
-		[CLKID_GEN]			= &gen.hw,
-		[CLKID_SARADC_SEL]		= &saradc_sel.hw,
-		[CLKID_SARADC]			= &saradc.hw,
-		[CLKID_PWM_A]			= &pwm_a.hw,
-		[CLKID_PWM_B]			= &pwm_b.hw,
-		[CLKID_PWM_C]			= &pwm_c.hw,
-		[CLKID_PWM_D]			= &pwm_d.hw,
-		[CLKID_PWM_E]			= &pwm_e.hw,
-		[CLKID_PWM_F]			= &pwm_f.hw,
-		[CLKID_SPICC]			= &spicc.hw,
-		[CLKID_TS]			= &ts.hw,
-		[CLKID_SPIFC]			= &spifc.hw,
-		[CLKID_USB_BUS]			= &usb_bus.hw,
-		[CLKID_SD_EMMC]			= &sd_emmc.hw,
-		[CLKID_PSRAM]			= &psram.hw,
-		[CLKID_DMC]			= &dmc.hw,
-		[CLKID_SYS_A_SEL]		= &sys_a_sel.hw,
-		[CLKID_SYS_A_DIV]		= &sys_a_div.hw,
-		[CLKID_SYS_A]			= &sys_a.hw,
-		[CLKID_SYS_B_SEL]		= &sys_b_sel.hw,
-		[CLKID_SYS_B_DIV]		= &sys_b_div.hw,
-		[CLKID_SYS_B]			= &sys_b.hw,
-		[CLKID_DSPA_A_SEL]		= &dspa_a_sel.hw,
-		[CLKID_DSPA_A_DIV]		= &dspa_a_div.hw,
-		[CLKID_DSPA_A]			= &dspa_a.hw,
-		[CLKID_DSPA_B_SEL]		= &dspa_b_sel.hw,
-		[CLKID_DSPA_B_DIV]		= &dspa_b_div.hw,
-		[CLKID_DSPA_B]			= &dspa_b.hw,
-		[CLKID_DSPB_A_SEL]		= &dspb_a_sel.hw,
-		[CLKID_DSPB_A_DIV]		= &dspb_a_div.hw,
-		[CLKID_DSPB_A]			= &dspb_a.hw,
-		[CLKID_DSPB_B_SEL]		= &dspb_b_sel.hw,
-		[CLKID_DSPB_B_DIV]		= &dspb_b_div.hw,
-		[CLKID_DSPB_B]			= &dspb_b.hw,
-		[CLKID_RTC_32K_IN]		= &rtc_32k_in.hw,
-		[CLKID_RTC_32K_DIV]		= &rtc_32k_div.hw,
-		[CLKID_RTC_32K_XTAL]		= &rtc_32k_xtal.hw,
-		[CLKID_RTC_32K_SEL]		= &rtc_32k_sel.hw,
-		[CLKID_CECB_32K_IN]		= &cecb_32k_in.hw,
-		[CLKID_CECB_32K_DIV]		= &cecb_32k_div.hw,
-		[CLKID_CECB_32K_SEL_PRE]	= &cecb_32k_sel_pre.hw,
-		[CLKID_CECB_32K_SEL]		= &cecb_32k_sel.hw,
-		[CLKID_CECA_32K_IN]		= &ceca_32k_in.hw,
-		[CLKID_CECA_32K_DIV]		= &ceca_32k_div.hw,
-		[CLKID_CECA_32K_SEL_PRE]	= &ceca_32k_sel_pre.hw,
-		[CLKID_CECA_32K_SEL]		= &ceca_32k_sel.hw,
-		[CLKID_DIV2_PRE]		= &fclk_div2_divn_pre.hw,
-		[CLKID_24M_DIV2]		= &clk_24m_div2.hw,
-		[CLKID_GEN_SEL]			= &gen_sel.hw,
-		[CLKID_GEN_DIV]			= &gen_div.hw,
-		[CLKID_SARADC_DIV]		= &saradc_div.hw,
-		[CLKID_PWM_A_SEL]		= &pwm_a_sel.hw,
-		[CLKID_PWM_A_DIV]		= &pwm_a_div.hw,
-		[CLKID_PWM_B_SEL]		= &pwm_b_sel.hw,
-		[CLKID_PWM_B_DIV]		= &pwm_b_div.hw,
-		[CLKID_PWM_C_SEL]		= &pwm_c_sel.hw,
-		[CLKID_PWM_C_DIV]		= &pwm_c_div.hw,
-		[CLKID_PWM_D_SEL]		= &pwm_d_sel.hw,
-		[CLKID_PWM_D_DIV]		= &pwm_d_div.hw,
-		[CLKID_PWM_E_SEL]		= &pwm_e_sel.hw,
-		[CLKID_PWM_E_DIV]		= &pwm_e_div.hw,
-		[CLKID_PWM_F_SEL]		= &pwm_f_sel.hw,
-		[CLKID_PWM_F_DIV]		= &pwm_f_div.hw,
-		[CLKID_SPICC_SEL]		= &spicc_sel.hw,
-		[CLKID_SPICC_DIV]		= &spicc_div.hw,
-		[CLKID_SPICC_SEL2]		= &spicc_sel2.hw,
-		[CLKID_TS_DIV]			= &ts_div.hw,
-		[CLKID_SPIFC_SEL]		= &spifc_sel.hw,
-		[CLKID_SPIFC_DIV]		= &spifc_div.hw,
-		[CLKID_SPIFC_SEL2]		= &spifc_sel2.hw,
-		[CLKID_USB_BUS_SEL]		= &usb_bus_sel.hw,
-		[CLKID_USB_BUS_DIV]		= &usb_bus_div.hw,
-		[CLKID_SD_EMMC_SEL]		= &sd_emmc_sel.hw,
-		[CLKID_SD_EMMC_DIV]		= &sd_emmc_div.hw,
-		[CLKID_SD_EMMC_SEL2]		= &sd_emmc_sel2.hw,
-		[CLKID_PSRAM_SEL]		= &psram_sel.hw,
-		[CLKID_PSRAM_DIV]		= &psram_div.hw,
-		[CLKID_PSRAM_SEL2]		= &psram_sel2.hw,
-		[CLKID_DMC_SEL]			= &dmc_sel.hw,
-		[CLKID_DMC_DIV]			= &dmc_div.hw,
-		[CLKID_DMC_SEL2]		= &dmc_sel2.hw,
-		[NR_CLKS]			= NULL,
-	},
-	.num = NR_CLKS,
+static struct clk_hw *a1_periphs_hw_clks[] = {
+	[CLKID_XTAL_IN]			= &xtal_in.hw,
+	[CLKID_FIXPLL_IN]		= &fixpll_in.hw,
+	[CLKID_USB_PHY_IN]		= &usb_phy_in.hw,
+	[CLKID_USB_CTRL_IN]		= &usb_ctrl_in.hw,
+	[CLKID_HIFIPLL_IN]		= &hifipll_in.hw,
+	[CLKID_SYSPLL_IN]		= &syspll_in.hw,
+	[CLKID_DDS_IN]			= &dds_in.hw,
+	[CLKID_SYS]			= &sys.hw,
+	[CLKID_CLKTREE]			= &clktree.hw,
+	[CLKID_RESET_CTRL]		= &reset_ctrl.hw,
+	[CLKID_ANALOG_CTRL]		= &analog_ctrl.hw,
+	[CLKID_PWR_CTRL]		= &pwr_ctrl.hw,
+	[CLKID_PAD_CTRL]		= &pad_ctrl.hw,
+	[CLKID_SYS_CTRL]		= &sys_ctrl.hw,
+	[CLKID_TEMP_SENSOR]		= &temp_sensor.hw,
+	[CLKID_AM2AXI_DIV]		= &am2axi_dev.hw,
+	[CLKID_SPICC_B]			= &spicc_b.hw,
+	[CLKID_SPICC_A]			= &spicc_a.hw,
+	[CLKID_MSR]			= &msr.hw,
+	[CLKID_AUDIO]			= &audio.hw,
+	[CLKID_JTAG_CTRL]		= &jtag_ctrl.hw,
+	[CLKID_SARADC_EN]		= &saradc_en.hw,
+	[CLKID_PWM_EF]			= &pwm_ef.hw,
+	[CLKID_PWM_CD]			= &pwm_cd.hw,
+	[CLKID_PWM_AB]			= &pwm_ab.hw,
+	[CLKID_CEC]			= &cec.hw,
+	[CLKID_I2C_S]			= &i2c_s.hw,
+	[CLKID_IR_CTRL]			= &ir_ctrl.hw,
+	[CLKID_I2C_M_D]			= &i2c_m_d.hw,
+	[CLKID_I2C_M_C]			= &i2c_m_c.hw,
+	[CLKID_I2C_M_B]			= &i2c_m_b.hw,
+	[CLKID_I2C_M_A]			= &i2c_m_a.hw,
+	[CLKID_ACODEC]			= &acodec.hw,
+	[CLKID_OTP]			= &otp.hw,
+	[CLKID_SD_EMMC_A]		= &sd_emmc_a.hw,
+	[CLKID_USB_PHY]			= &usb_phy.hw,
+	[CLKID_USB_CTRL]		= &usb_ctrl.hw,
+	[CLKID_SYS_DSPB]		= &sys_dspb.hw,
+	[CLKID_SYS_DSPA]		= &sys_dspa.hw,
+	[CLKID_DMA]			= &dma.hw,
+	[CLKID_IRQ_CTRL]		= &irq_ctrl.hw,
+	[CLKID_NIC]			= &nic.hw,
+	[CLKID_GIC]			= &gic.hw,
+	[CLKID_UART_C]			= &uart_c.hw,
+	[CLKID_UART_B]			= &uart_b.hw,
+	[CLKID_UART_A]			= &uart_a.hw,
+	[CLKID_SYS_PSRAM]		= &sys_psram.hw,
+	[CLKID_RSA]			= &rsa.hw,
+	[CLKID_CORESIGHT]		= &coresight.hw,
+	[CLKID_AM2AXI_VAD]		= &am2axi_vad.hw,
+	[CLKID_AUDIO_VAD]		= &audio_vad.hw,
+	[CLKID_AXI_DMC]			= &axi_dmc.hw,
+	[CLKID_AXI_PSRAM]		= &axi_psram.hw,
+	[CLKID_RAMB]			= &ramb.hw,
+	[CLKID_RAMA]			= &rama.hw,
+	[CLKID_AXI_SPIFC]		= &axi_spifc.hw,
+	[CLKID_AXI_NIC]			= &axi_nic.hw,
+	[CLKID_AXI_DMA]			= &axi_dma.hw,
+	[CLKID_CPU_CTRL]		= &cpu_ctrl.hw,
+	[CLKID_ROM]			= &rom.hw,
+	[CLKID_PROC_I2C]		= &prod_i2c.hw,
+	[CLKID_DSPA_SEL]		= &dspa_sel.hw,
+	[CLKID_DSPB_SEL]		= &dspb_sel.hw,
+	[CLKID_DSPA_EN]			= &dspa_en.hw,
+	[CLKID_DSPA_EN_NIC]		= &dspa_en_nic.hw,
+	[CLKID_DSPB_EN]			= &dspb_en.hw,
+	[CLKID_DSPB_EN_NIC]		= &dspb_en_nic.hw,
+	[CLKID_RTC]			= &rtc.hw,
+	[CLKID_CECA_32K]		= &ceca_32k_out.hw,
+	[CLKID_CECB_32K]		= &cecb_32k_out.hw,
+	[CLKID_24M]			= &clk_24m.hw,
+	[CLKID_12M]			= &clk_12m.hw,
+	[CLKID_FCLK_DIV2_DIVN]		= &fclk_div2_divn.hw,
+	[CLKID_GEN]			= &gen.hw,
+	[CLKID_SARADC_SEL]		= &saradc_sel.hw,
+	[CLKID_SARADC]			= &saradc.hw,
+	[CLKID_PWM_A]			= &pwm_a.hw,
+	[CLKID_PWM_B]			= &pwm_b.hw,
+	[CLKID_PWM_C]			= &pwm_c.hw,
+	[CLKID_PWM_D]			= &pwm_d.hw,
+	[CLKID_PWM_E]			= &pwm_e.hw,
+	[CLKID_PWM_F]			= &pwm_f.hw,
+	[CLKID_SPICC]			= &spicc.hw,
+	[CLKID_TS]			= &ts.hw,
+	[CLKID_SPIFC]			= &spifc.hw,
+	[CLKID_USB_BUS]			= &usb_bus.hw,
+	[CLKID_SD_EMMC]			= &sd_emmc.hw,
+	[CLKID_PSRAM]			= &psram.hw,
+	[CLKID_DMC]			= &dmc.hw,
+	[CLKID_SYS_A_SEL]		= &sys_a_sel.hw,
+	[CLKID_SYS_A_DIV]		= &sys_a_div.hw,
+	[CLKID_SYS_A]			= &sys_a.hw,
+	[CLKID_SYS_B_SEL]		= &sys_b_sel.hw,
+	[CLKID_SYS_B_DIV]		= &sys_b_div.hw,
+	[CLKID_SYS_B]			= &sys_b.hw,
+	[CLKID_DSPA_A_SEL]		= &dspa_a_sel.hw,
+	[CLKID_DSPA_A_DIV]		= &dspa_a_div.hw,
+	[CLKID_DSPA_A]			= &dspa_a.hw,
+	[CLKID_DSPA_B_SEL]		= &dspa_b_sel.hw,
+	[CLKID_DSPA_B_DIV]		= &dspa_b_div.hw,
+	[CLKID_DSPA_B]			= &dspa_b.hw,
+	[CLKID_DSPB_A_SEL]		= &dspb_a_sel.hw,
+	[CLKID_DSPB_A_DIV]		= &dspb_a_div.hw,
+	[CLKID_DSPB_A]			= &dspb_a.hw,
+	[CLKID_DSPB_B_SEL]		= &dspb_b_sel.hw,
+	[CLKID_DSPB_B_DIV]		= &dspb_b_div.hw,
+	[CLKID_DSPB_B]			= &dspb_b.hw,
+	[CLKID_RTC_32K_IN]		= &rtc_32k_in.hw,
+	[CLKID_RTC_32K_DIV]		= &rtc_32k_div.hw,
+	[CLKID_RTC_32K_XTAL]		= &rtc_32k_xtal.hw,
+	[CLKID_RTC_32K_SEL]		= &rtc_32k_sel.hw,
+	[CLKID_CECB_32K_IN]		= &cecb_32k_in.hw,
+	[CLKID_CECB_32K_DIV]		= &cecb_32k_div.hw,
+	[CLKID_CECB_32K_SEL_PRE]	= &cecb_32k_sel_pre.hw,
+	[CLKID_CECB_32K_SEL]		= &cecb_32k_sel.hw,
+	[CLKID_CECA_32K_IN]		= &ceca_32k_in.hw,
+	[CLKID_CECA_32K_DIV]		= &ceca_32k_div.hw,
+	[CLKID_CECA_32K_SEL_PRE]	= &ceca_32k_sel_pre.hw,
+	[CLKID_CECA_32K_SEL]		= &ceca_32k_sel.hw,
+	[CLKID_DIV2_PRE]		= &fclk_div2_divn_pre.hw,
+	[CLKID_24M_DIV2]		= &clk_24m_div2.hw,
+	[CLKID_GEN_SEL]			= &gen_sel.hw,
+	[CLKID_GEN_DIV]			= &gen_div.hw,
+	[CLKID_SARADC_DIV]		= &saradc_div.hw,
+	[CLKID_PWM_A_SEL]		= &pwm_a_sel.hw,
+	[CLKID_PWM_A_DIV]		= &pwm_a_div.hw,
+	[CLKID_PWM_B_SEL]		= &pwm_b_sel.hw,
+	[CLKID_PWM_B_DIV]		= &pwm_b_div.hw,
+	[CLKID_PWM_C_SEL]		= &pwm_c_sel.hw,
+	[CLKID_PWM_C_DIV]		= &pwm_c_div.hw,
+	[CLKID_PWM_D_SEL]		= &pwm_d_sel.hw,
+	[CLKID_PWM_D_DIV]		= &pwm_d_div.hw,
+	[CLKID_PWM_E_SEL]		= &pwm_e_sel.hw,
+	[CLKID_PWM_E_DIV]		= &pwm_e_div.hw,
+	[CLKID_PWM_F_SEL]		= &pwm_f_sel.hw,
+	[CLKID_PWM_F_DIV]		= &pwm_f_div.hw,
+	[CLKID_SPICC_SEL]		= &spicc_sel.hw,
+	[CLKID_SPICC_DIV]		= &spicc_div.hw,
+	[CLKID_SPICC_SEL2]		= &spicc_sel2.hw,
+	[CLKID_TS_DIV]			= &ts_div.hw,
+	[CLKID_SPIFC_SEL]		= &spifc_sel.hw,
+	[CLKID_SPIFC_DIV]		= &spifc_div.hw,
+	[CLKID_SPIFC_SEL2]		= &spifc_sel2.hw,
+	[CLKID_USB_BUS_SEL]		= &usb_bus_sel.hw,
+	[CLKID_USB_BUS_DIV]		= &usb_bus_div.hw,
+	[CLKID_SD_EMMC_SEL]		= &sd_emmc_sel.hw,
+	[CLKID_SD_EMMC_DIV]		= &sd_emmc_div.hw,
+	[CLKID_SD_EMMC_SEL2]		= &sd_emmc_sel2.hw,
+	[CLKID_PSRAM_SEL]		= &psram_sel.hw,
+	[CLKID_PSRAM_DIV]		= &psram_div.hw,
+	[CLKID_PSRAM_SEL2]		= &psram_sel2.hw,
+	[CLKID_DMC_SEL]			= &dmc_sel.hw,
+	[CLKID_DMC_DIV]			= &dmc_div.hw,
+	[CLKID_DMC_SEL2]		= &dmc_sel2.hw,
 };
 
 /* Convenience table to populate regmap in .probe */
@@ -2190,6 +2186,29 @@  static struct regmap_config a1_periphs_regmap_cfg = {
 	.reg_stride = 4,
 };
 
+struct meson_a1_periphs_clks {
+	struct clk_hw **hw_clks;
+	unsigned int hw_clk_num;
+};
+
+static struct meson_a1_periphs_clks a1_periphs_clks = {
+	.hw_clks = a1_periphs_hw_clks,
+	.hw_clk_num = ARRAY_SIZE(a1_periphs_hw_clks),
+};
+
+static struct clk_hw *meson_a1_periphs_hw_get(struct of_phandle_args *clkspec, void *clk_data)
+{
+	const struct meson_a1_periphs_clks *data = clk_data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx >= data->hw_clk_num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return data->hw_clks[idx];
+}
+
 static int meson_a1_periphs_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -2211,15 +2230,15 @@  static int meson_a1_periphs_probe(struct platform_device *pdev)
 	for (i = 0; i < ARRAY_SIZE(a1_periphs_regmaps); i++)
 		a1_periphs_regmaps[i]->map = map;
 
-	for (clkid = 0; clkid < a1_periphs_clks.num; clkid++) {
-		err = devm_clk_hw_register(dev, a1_periphs_clks.hws[clkid]);
+	for (clkid = 0; clkid < a1_periphs_clks.hw_clk_num; clkid++) {
+		err = devm_clk_hw_register(dev, a1_periphs_clks.hw_clks[clkid]);
 		if (err)
 			return dev_err_probe(dev, err,
 					     "clock[%d] registration failed\n",
 					     clkid);
 	}
 
-	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+	return devm_of_clk_add_hw_provider(dev, meson_a1_periphs_hw_get,
 					   &a1_periphs_clks);
 }
 
diff --git a/drivers/clk/meson/a1-peripherals.h b/drivers/clk/meson/a1-peripherals.h
index 526fc9ba5c9f..4d60456a95a9 100644
--- a/drivers/clk/meson/a1-peripherals.h
+++ b/drivers/clk/meson/a1-peripherals.h
@@ -108,6 +108,5 @@ 
 #define CLKID_DMC_SEL		151
 #define CLKID_DMC_DIV		152
 #define CLKID_DMC_SEL2		153
-#define NR_CLKS			154
 
 #endif /* __A1_PERIPHERALS_H */
diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
index bd2f1d1ec6e4..25d102dc8a5d 100644
--- a/drivers/clk/meson/a1-pll.c
+++ b/drivers/clk/meson/a1-pll.c
@@ -268,22 +268,18 @@  static struct clk_regmap fclk_div7 = {
 };
 
 /* Array of all clocks registered by this provider */
-static struct clk_hw_onecell_data a1_pll_clks = {
-	.hws = {
-		[CLKID_FIXED_PLL_DCO]	= &fixed_pll_dco.hw,
-		[CLKID_FIXED_PLL]	= &fixed_pll.hw,
-		[CLKID_FCLK_DIV2_DIV]	= &fclk_div2_div.hw,
-		[CLKID_FCLK_DIV3_DIV]	= &fclk_div3_div.hw,
-		[CLKID_FCLK_DIV5_DIV]	= &fclk_div5_div.hw,
-		[CLKID_FCLK_DIV7_DIV]	= &fclk_div7_div.hw,
-		[CLKID_FCLK_DIV2]	= &fclk_div2.hw,
-		[CLKID_FCLK_DIV3]	= &fclk_div3.hw,
-		[CLKID_FCLK_DIV5]	= &fclk_div5.hw,
-		[CLKID_FCLK_DIV7]	= &fclk_div7.hw,
-		[CLKID_HIFI_PLL]	= &hifi_pll.hw,
-		[NR_PLL_CLKS]		= NULL,
-	},
-	.num = NR_PLL_CLKS,
+static struct clk_hw *a1_pll_hw_clks[] = {
+	[CLKID_FIXED_PLL_DCO]	= &fixed_pll_dco.hw,
+	[CLKID_FIXED_PLL]	= &fixed_pll.hw,
+	[CLKID_FCLK_DIV2_DIV]	= &fclk_div2_div.hw,
+	[CLKID_FCLK_DIV3_DIV]	= &fclk_div3_div.hw,
+	[CLKID_FCLK_DIV5_DIV]	= &fclk_div5_div.hw,
+	[CLKID_FCLK_DIV7_DIV]	= &fclk_div7_div.hw,
+	[CLKID_FCLK_DIV2]	= &fclk_div2.hw,
+	[CLKID_FCLK_DIV3]	= &fclk_div3.hw,
+	[CLKID_FCLK_DIV5]	= &fclk_div5.hw,
+	[CLKID_FCLK_DIV7]	= &fclk_div7.hw,
+	[CLKID_HIFI_PLL]	= &hifi_pll.hw,
 };
 
 static struct clk_regmap *const a1_pll_regmaps[] = {
@@ -302,6 +298,29 @@  static struct regmap_config a1_pll_regmap_cfg = {
 	.reg_stride = 4,
 };
 
+struct meson_a1_pll_clks {
+	struct clk_hw **hw_clks;
+	unsigned int hw_clk_num;
+};
+
+static struct meson_a1_pll_clks a1_pll_clks = {
+	.hw_clks = a1_pll_hw_clks,
+	.hw_clk_num = ARRAY_SIZE(a1_pll_hw_clks),
+};
+
+static struct clk_hw *meson_a1_pll_hw_get(struct of_phandle_args *clkspec, void *clk_data)
+{
+	const struct meson_a1_pll_clks *data = clk_data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx >= data->hw_clk_num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return data->hw_clks[idx];
+}
+
 static int meson_a1_pll_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -324,15 +343,15 @@  static int meson_a1_pll_probe(struct platform_device *pdev)
 		a1_pll_regmaps[i]->map = map;
 
 	/* Register clocks */
-	for (clkid = 0; clkid < a1_pll_clks.num; clkid++) {
-		err = devm_clk_hw_register(dev, a1_pll_clks.hws[clkid]);
+	for (clkid = 0; clkid < a1_pll_clks.hw_clk_num; clkid++) {
+		err = devm_clk_hw_register(dev, a1_pll_clks.hw_clks[clkid]);
 		if (err)
 			return dev_err_probe(dev, err,
 					     "clock[%d] registration failed\n",
 					     clkid);
 	}
 
-	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+	return devm_of_clk_add_hw_provider(dev, meson_a1_pll_hw_get,
 					   &a1_pll_clks);
 }
 
diff --git a/drivers/clk/meson/a1-pll.h b/drivers/clk/meson/a1-pll.h
index 29726651b056..82570759e6a2 100644
--- a/drivers/clk/meson/a1-pll.h
+++ b/drivers/clk/meson/a1-pll.h
@@ -42,6 +42,5 @@ 
 #define CLKID_FCLK_DIV3_DIV	3
 #define CLKID_FCLK_DIV5_DIV	4
 #define CLKID_FCLK_DIV7_DIV	5
-#define NR_PLL_CLKS		11
 
 #endif /* __A1_PLL_H */