diff mbox series

[RFC,v2,1/5] clk: meson: axg: move reset controller's code to separate module

Message ID 20240328010831.884487-2-jan.dakinevich@salutedevices.com (mailing list archive)
State Superseded, archived
Headers show
Series Add A1 Soc audio clock controller driver | expand

Commit Message

Jan Dakinevich March 28, 2024, 1:08 a.m. UTC
This code will by reused by A1 SoC.

Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
---
 drivers/clk/meson/Kconfig            |   5 ++
 drivers/clk/meson/Makefile           |   1 +
 drivers/clk/meson/axg-audio.c        |  95 +----------------------
 drivers/clk/meson/meson-audio-rstc.c | 109 +++++++++++++++++++++++++++
 drivers/clk/meson/meson-audio-rstc.h |  12 +++
 5 files changed, 130 insertions(+), 92 deletions(-)
 create mode 100644 drivers/clk/meson/meson-audio-rstc.c
 create mode 100644 drivers/clk/meson/meson-audio-rstc.h

Comments

Jerome Brunet April 2, 2024, 2:52 p.m. UTC | #1
On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:

> This code will by reused by A1 SoC.

Could expand a bit please ?

>
> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>

In general, I like the idea.

We do have a couple a reset registers lost in middle of clocks and this
change makes it possible to re-use the code instead duplicating it.

The exported function would be used by audio clock controllers, but the
module created would be purely about reset.

One may wonder how it ended up in the clock tree, especially since the
kernel as a reset tree too.

I'm not sure if this should move to the reset framework or if it would
be an unnecessary churn. Stephen, Philipp, do you have an opinion on
this ?

> ---
>  drivers/clk/meson/Kconfig            |   5 ++
>  drivers/clk/meson/Makefile           |   1 +
>  drivers/clk/meson/axg-audio.c        |  95 +----------------------
>  drivers/clk/meson/meson-audio-rstc.c | 109 +++++++++++++++++++++++++++
>  drivers/clk/meson/meson-audio-rstc.h |  12 +++
>  5 files changed, 130 insertions(+), 92 deletions(-)
>  create mode 100644 drivers/clk/meson/meson-audio-rstc.c
>  create mode 100644 drivers/clk/meson/meson-audio-rstc.h
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 29ffd14d267b..d6a2fa5f7e88 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -48,6 +48,10 @@ config COMMON_CLK_MESON_CPU_DYNDIV
>  	tristate
>  	select COMMON_CLK_MESON_REGMAP
>  
> +config COMMON_CLK_MESON_AUDIO_RSTC
> +	tristate
> +	select RESET_CONTROLLER
> +
>  config COMMON_CLK_MESON8B
>  	bool "Meson8 SoC Clock controller support"
>  	depends on ARM
> @@ -101,6 +105,7 @@ config COMMON_CLK_AXG_AUDIO
>  	select COMMON_CLK_MESON_PHASE
>  	select COMMON_CLK_MESON_SCLK_DIV
>  	select COMMON_CLK_MESON_CLKC_UTILS
> +	select COMMON_CLK_MESON_AUDIO_RSTC
>  	select REGMAP_MMIO
>  	help
>  	  Support for the audio clock controller on AmLogic A113D devices,
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 9ee4b954c896..88d94921a4dc 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>  obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>  obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>  obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
> +obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
>  
>  # Amlogic Clock controllers
>  
> diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
> index ac3482960903..990203a7ad5c 100644
> --- a/drivers/clk/meson/axg-audio.c
> +++ b/drivers/clk/meson/axg-audio.c
> @@ -12,10 +12,10 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
> -#include <linux/reset-controller.h>
>  #include <linux/slab.h>
>  
>  #include "meson-clkc-utils.h"
> +#include "meson-audio-rstc.h"
>  #include "axg-audio.h"
>  #include "clk-regmap.h"
>  #include "clk-phase.h"
> @@ -1648,84 +1648,6 @@ static struct clk_regmap *const sm1_clk_regmaps[] = {
>  	&sm1_sysclk_b_en,
>  };
>  
> -struct axg_audio_reset_data {
> -	struct reset_controller_dev rstc;
> -	struct regmap *map;
> -	unsigned int offset;
> -};
> -
> -static void axg_audio_reset_reg_and_bit(struct axg_audio_reset_data *rst,
> -					unsigned long id,
> -					unsigned int *reg,
> -					unsigned int *bit)
> -{
> -	unsigned int stride = regmap_get_reg_stride(rst->map);
> -
> -	*reg = (id / (stride * BITS_PER_BYTE)) * stride;
> -	*reg += rst->offset;
> -	*bit = id % (stride * BITS_PER_BYTE);
> -}
> -
> -static int axg_audio_reset_update(struct reset_controller_dev *rcdev,
> -				unsigned long id, bool assert)
> -{
> -	struct axg_audio_reset_data *rst =
> -		container_of(rcdev, struct axg_audio_reset_data, rstc);
> -	unsigned int offset, bit;
> -
> -	axg_audio_reset_reg_and_bit(rst, id, &offset, &bit);
> -
> -	regmap_update_bits(rst->map, offset, BIT(bit),
> -			assert ? BIT(bit) : 0);
> -
> -	return 0;
> -}
> -
> -static int axg_audio_reset_status(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	struct axg_audio_reset_data *rst =
> -		container_of(rcdev, struct axg_audio_reset_data, rstc);
> -	unsigned int val, offset, bit;
> -
> -	axg_audio_reset_reg_and_bit(rst, id, &offset, &bit);
> -
> -	regmap_read(rst->map, offset, &val);
> -
> -	return !!(val & BIT(bit));
> -}
> -
> -static int axg_audio_reset_assert(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	return axg_audio_reset_update(rcdev, id, true);
> -}
> -
> -static int axg_audio_reset_deassert(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	return axg_audio_reset_update(rcdev, id, false);
> -}
> -
> -static int axg_audio_reset_toggle(struct reset_controller_dev *rcdev,
> -				unsigned long id)
> -{
> -	int ret;
> -
> -	ret = axg_audio_reset_assert(rcdev, id);
> -	if (ret)
> -		return ret;
> -
> -	return axg_audio_reset_deassert(rcdev, id);
> -}
> -
> -static const struct reset_control_ops axg_audio_rstc_ops = {
> -	.assert = axg_audio_reset_assert,
> -	.deassert = axg_audio_reset_deassert,
> -	.reset = axg_audio_reset_toggle,
> -	.status = axg_audio_reset_status,
> -};
> -
>  static const struct regmap_config axg_audio_regmap_cfg = {
>  	.reg_bits	= 32,
>  	.val_bits	= 32,
> @@ -1745,7 +1667,6 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	const struct audioclk_data *data;
> -	struct axg_audio_reset_data *rst;
>  	struct regmap *map;
>  	void __iomem *regs;
>  	struct clk_hw *hw;
> @@ -1807,18 +1728,8 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
>  	if (!data->reset_num)
>  		return 0;
>  
> -	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> -	if (!rst)
> -		return -ENOMEM;
> -
> -	rst->map = map;
> -	rst->offset = data->reset_offset;
> -	rst->rstc.nr_resets = data->reset_num;
> -	rst->rstc.ops = &axg_audio_rstc_ops;
> -	rst->rstc.of_node = dev->of_node;
> -	rst->rstc.owner = THIS_MODULE;
> -
> -	return devm_reset_controller_register(dev, &rst->rstc);
> +	return meson_audio_rstc_register(dev, map, data->reset_offset,
> +					 data->reset_num);
>  }
>  
>  static const struct audioclk_data axg_audioclk_data = {
> diff --git a/drivers/clk/meson/meson-audio-rstc.c b/drivers/clk/meson/meson-audio-rstc.c
> new file mode 100644
> index 000000000000..2079d24c40f4
> --- /dev/null
> +++ b/drivers/clk/meson/meson-audio-rstc.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2018 BayLibre, SAS.
> + * Author: Jerome Brunet <jbrunet@baylibre.com>
> + */
> +
> +#include <linux/reset-controller.h>
> +
> +#include "meson-audio-rstc.h"
> +
> +struct meson_audio_reset_data {
> +	struct reset_controller_dev rstc;
> +	struct regmap *map;
> +	unsigned int offset;
> +};
> +
> +static void meson_audio_reset_reg_and_bit(struct meson_audio_reset_data *rst,
> +					  unsigned long id,
> +					  unsigned int *reg,
> +					  unsigned int *bit)
> +{
> +	unsigned int stride = regmap_get_reg_stride(rst->map);
> +
> +	*reg = (id / (stride * BITS_PER_BYTE)) * stride;
> +	*reg += rst->offset;
> +	*bit = id % (stride * BITS_PER_BYTE);
> +}
> +
> +static int meson_audio_reset_update(struct reset_controller_dev *rcdev,
> +				    unsigned long id, bool assert)
> +{
> +	struct meson_audio_reset_data *rst =
> +		container_of(rcdev, struct meson_audio_reset_data, rstc);
> +	unsigned int offset, bit;
> +
> +	meson_audio_reset_reg_and_bit(rst, id, &offset, &bit);
> +
> +	regmap_update_bits(rst->map, offset, BIT(bit),
> +			assert ? BIT(bit) : 0);
> +
> +	return 0;
> +}
> +
> +static int meson_audio_reset_status(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	struct meson_audio_reset_data *rst =
> +		container_of(rcdev, struct meson_audio_reset_data, rstc);
> +	unsigned int val, offset, bit;
> +
> +	meson_audio_reset_reg_and_bit(rst, id, &offset, &bit);
> +
> +	regmap_read(rst->map, offset, &val);
> +
> +	return !!(val & BIT(bit));
> +}
> +
> +static int meson_audio_reset_assert(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	return meson_audio_reset_update(rcdev, id, true);
> +}
> +
> +static int meson_audio_reset_deassert(struct reset_controller_dev *rcdev,
> +				      unsigned long id)
> +{
> +	return meson_audio_reset_update(rcdev, id, false);
> +}
> +
> +static int meson_audio_reset_toggle(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	int ret;
> +
> +	ret = meson_audio_reset_assert(rcdev, id);
> +	if (ret)
> +		return ret;
> +
> +	return meson_audio_reset_deassert(rcdev, id);
> +}
> +
> +static const struct reset_control_ops meson_audio_rstc_ops = {
> +	.assert = meson_audio_reset_assert,
> +	.deassert = meson_audio_reset_deassert,
> +	.reset = meson_audio_reset_toggle,
> +	.status = meson_audio_reset_status,
> +};
> +
> +int meson_audio_rstc_register(struct device *dev, struct regmap *map,
> +			      unsigned int offset, unsigned int num)
> +{
> +	struct meson_audio_reset_data *rst;
> +
> +	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> +	if (!rst)
> +		return -ENOMEM;
> +
> +	rst->map = map;
> +	rst->offset = offset;
> +	rst->rstc.nr_resets = num;
> +	rst->rstc.ops = &meson_audio_rstc_ops;
> +	rst->rstc.of_node = dev->of_node;
> +	rst->rstc.owner = THIS_MODULE;
> +
> +	return devm_reset_controller_register(dev, &rst->rstc);
> +}
> +EXPORT_SYMBOL_GPL(meson_audio_rstc_register);
> +
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/meson/meson-audio-rstc.h b/drivers/clk/meson/meson-audio-rstc.h
> new file mode 100644
> index 000000000000..6b441549de03
> --- /dev/null
> +++ b/drivers/clk/meson/meson-audio-rstc.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +
> +#ifndef __MESON_AUDIO_RSTC_H
> +#define __MESON_AUDIO_RSTC_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +
> +int meson_audio_rstc_register(struct device *dev, struct regmap *map,
> +			      unsigned int offset, unsigned int num);
> +
> +#endif /* __MESON_AUDIO_RSTC_H */
Stephen Boyd April 8, 2024, 2:39 a.m. UTC | #2
Quoting Jerome Brunet (2024-04-02 07:52:38)
> 
> On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> 
> > This code will by reused by A1 SoC.
> 
> Could expand a bit please ?
> 
> >
> > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> 
> In general, I like the idea.
> 
> We do have a couple a reset registers lost in middle of clocks and this
> change makes it possible to re-use the code instead duplicating it.
> 
> The exported function would be used by audio clock controllers, but the
> module created would be purely about reset.
> 
> One may wonder how it ended up in the clock tree, especially since the
> kernel as a reset tree too.
> 
> I'm not sure if this should move to the reset framework or if it would
> be an unnecessary churn. Stephen, Philipp, do you have an opinion on
> this ?
> 

I'd prefer it be made into an auxiliary device and the driver put in
drivers/reset/ so we can keep reset code in the reset directory. The
auxiliary device creation function can also be in the drivers/reset/
directory so that the clk driver calls some function to create and
register the device.
Philipp Zabel April 8, 2024, 8:21 a.m. UTC | #3
On So, 2024-04-07 at 19:39 -0700, Stephen Boyd wrote:
> Quoting Jerome Brunet (2024-04-02 07:52:38)
> > 
> > On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> > 
> > > This code will by reused by A1 SoC.
> > 
> > Could expand a bit please ?
> > 
> > > 
> > > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> > 
> > In general, I like the idea.
> > 
> > We do have a couple a reset registers lost in middle of clocks and this
> > change makes it possible to re-use the code instead duplicating it.
> > 
> > The exported function would be used by audio clock controllers, but the
> > module created would be purely about reset.
> > 
> > One may wonder how it ended up in the clock tree, especially since the
> > kernel as a reset tree too.
> > 
> > I'm not sure if this should move to the reset framework or if it would
> > be an unnecessary churn. Stephen, Philipp, do you have an opinion on
> > this ?
> > 
> 
> I'd prefer it be made into an auxiliary device and the driver put in
> drivers/reset/ so we can keep reset code in the reset directory.

Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
starfive-jh7110 drivers are examples of this.

> The auxiliary device creation function can also be in the
> drivers/reset/ directory so that the clk driver calls some function
> to create and register the device.

I'm undecided about this, do you think mpfs_reset_controller_register()
and jh7110_reset_controller_register() should rather live with the
reset aux drivers in drivers/reset/ ?

regards
Philipp
Stephen Boyd April 8, 2024, 9:52 a.m. UTC | #4
Quoting Philipp Zabel (2024-04-08 01:21:47)
> On So, 2024-04-07 at 19:39 -0700, Stephen Boyd wrote:
> > Quoting Jerome Brunet (2024-04-02 07:52:38)
> > > 
> > > On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> > > 
> > > > This code will by reused by A1 SoC.
> > > 
> > > Could expand a bit please ?
> > > 
> > > > 
> > > > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> > > 
> > > In general, I like the idea.
> > > 
> > > We do have a couple a reset registers lost in middle of clocks and this
> > > change makes it possible to re-use the code instead duplicating it.
> > > 
> > > The exported function would be used by audio clock controllers, but the
> > > module created would be purely about reset.
> > > 
> > > One may wonder how it ended up in the clock tree, especially since the
> > > kernel as a reset tree too.
> > > 
> > > I'm not sure if this should move to the reset framework or if it would
> > > be an unnecessary churn. Stephen, Philipp, do you have an opinion on
> > > this ?
> > > 
> > 
> > I'd prefer it be made into an auxiliary device and the driver put in
> > drivers/reset/ so we can keep reset code in the reset directory.
> 
> Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> starfive-jh7110 drivers are examples of this.
> 
> > The auxiliary device creation function can also be in the
> > drivers/reset/ directory so that the clk driver calls some function
> > to create and register the device.
> 
> I'm undecided about this, do you think mpfs_reset_controller_register()
> and jh7110_reset_controller_register() should rather live with the
> reset aux drivers in drivers/reset/ ?

Yes, and also mpfs_reset_read() and friends. We should pass the base
iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
then move all that code into drivers/reset with some header file
exported function to call. That way the clk driver hands over the data
without having to implement half the implementation.
Conor Dooley April 8, 2024, 5:05 p.m. UTC | #5
On Mon, Apr 08, 2024 at 02:52:37AM -0700, Stephen Boyd wrote:
> Quoting Philipp Zabel (2024-04-08 01:21:47)
> > On So, 2024-04-07 at 19:39 -0700, Stephen Boyd wrote:
> > > Quoting Jerome Brunet (2024-04-02 07:52:38)
> > > > 
> > > > On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> > > > 
> > > > > This code will by reused by A1 SoC.
> > > > 
> > > > Could expand a bit please ?
> > > > 
> > > > > 
> > > > > Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> > > > 
> > > > In general, I like the idea.
> > > > 
> > > > We do have a couple a reset registers lost in middle of clocks and this
> > > > change makes it possible to re-use the code instead duplicating it.
> > > > 
> > > > The exported function would be used by audio clock controllers, but the
> > > > module created would be purely about reset.
> > > > 
> > > > One may wonder how it ended up in the clock tree, especially since the
> > > > kernel as a reset tree too.
> > > > 
> > > > I'm not sure if this should move to the reset framework or if it would
> > > > be an unnecessary churn. Stephen, Philipp, do you have an opinion on
> > > > this ?
> > > > 
> > > 
> > > I'd prefer it be made into an auxiliary device and the driver put in
> > > drivers/reset/ so we can keep reset code in the reset directory.
> > 
> > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > starfive-jh7110 drivers are examples of this.
> > 
> > > The auxiliary device creation function can also be in the
> > > drivers/reset/ directory so that the clk driver calls some function
> > > to create and register the device.
> > 
> > I'm undecided about this, do you think mpfs_reset_controller_register()
> > and jh7110_reset_controller_register() should rather live with the
> > reset aux drivers in drivers/reset/ ?
> 
> Yes, and also mpfs_reset_read() and friends. We should pass the base
> iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> then move all that code into drivers/reset with some header file
> exported function to call. That way the clk driver hands over the data
> without having to implement half the implementation.

I'll todo list that :)
Conor Dooley April 9, 2024, 12:05 p.m. UTC | #6
On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:

> > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > > starfive-jh7110 drivers are examples of this.
> > > 
> > > > The auxiliary device creation function can also be in the
> > > > drivers/reset/ directory so that the clk driver calls some function
> > > > to create and register the device.
> > > 
> > > I'm undecided about this, do you think mpfs_reset_controller_register()
> > > and jh7110_reset_controller_register() should rather live with the
> > > reset aux drivers in drivers/reset/ ?
> > 
> > Yes, and also mpfs_reset_read() and friends. We should pass the base
> > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> > then move all that code into drivers/reset with some header file
> > exported function to call. That way the clk driver hands over the data
> > without having to implement half the implementation.
> 
> I'll todo list that :)

Something like the below?

-- >8 --
From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
From: Conor Dooley <conor.dooley@microchip.com>
Date: Tue, 9 Apr 2024 11:54:34 +0100
Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
 reset subsystem

<insert something here>

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/clk-mpfs.c | 90 +-------------------------------
 drivers/reset/reset-mpfs.c       | 74 +++++++++++++++++++++++---
 include/soc/microchip/mpfs.h     | 10 ++--
 3 files changed, 73 insertions(+), 101 deletions(-)

diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index 22eab91a6712..432080c35cec 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -4,7 +4,6 @@
  *
  * Copyright (C) 2020-2022 Microchip Technology Inc. All rights reserved.
  */
-#include <linux/auxiliary_bus.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -361,93 +360,6 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
 	return 0;
 }
 
-/*
- * Peripheral clock resets
- */
-
-#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
-
-u32 mpfs_reset_read(struct device *dev)
-{
-	struct mpfs_clock_data *clock_data = dev_get_drvdata(dev->parent);
-
-	return readl_relaxed(clock_data->base + REG_SUBBLK_RESET_CR);
-}
-EXPORT_SYMBOL_NS_GPL(mpfs_reset_read, MCHP_CLK_MPFS);
-
-void mpfs_reset_write(struct device *dev, u32 val)
-{
-	struct mpfs_clock_data *clock_data = dev_get_drvdata(dev->parent);
-
-	writel_relaxed(val, clock_data->base + REG_SUBBLK_RESET_CR);
-}
-EXPORT_SYMBOL_NS_GPL(mpfs_reset_write, MCHP_CLK_MPFS);
-
-static void mpfs_reset_unregister_adev(void *_adev)
-{
-	struct auxiliary_device *adev = _adev;
-
-	auxiliary_device_delete(adev);
-	auxiliary_device_uninit(adev);
-}
-
-static void mpfs_reset_adev_release(struct device *dev)
-{
-	struct auxiliary_device *adev = to_auxiliary_dev(dev);
-
-	kfree(adev);
-}
-
-static struct auxiliary_device *mpfs_reset_adev_alloc(struct mpfs_clock_data *clk_data)
-{
-	struct auxiliary_device *adev;
-	int ret;
-
-	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
-	if (!adev)
-		return ERR_PTR(-ENOMEM);
-
-	adev->name = "reset-mpfs";
-	adev->dev.parent = clk_data->dev;
-	adev->dev.release = mpfs_reset_adev_release;
-	adev->id = 666u;
-
-	ret = auxiliary_device_init(adev);
-	if (ret) {
-		kfree(adev);
-		return ERR_PTR(ret);
-	}
-
-	return adev;
-}
-
-static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
-{
-	struct auxiliary_device *adev;
-	int ret;
-
-	adev = mpfs_reset_adev_alloc(clk_data);
-	if (IS_ERR(adev))
-		return PTR_ERR(adev);
-
-	ret = auxiliary_device_add(adev);
-	if (ret) {
-		auxiliary_device_uninit(adev);
-		return ret;
-	}
-
-	return devm_add_action_or_reset(clk_data->dev, mpfs_reset_unregister_adev, adev);
-}
-
-#else /* !CONFIG_RESET_CONTROLLER */
-
-static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
-{
-	return 0;
-}
-
-#endif /* !CONFIG_RESET_CONTROLLER */
-
 static int mpfs_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -499,7 +411,7 @@ static int mpfs_clk_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return mpfs_reset_controller_register(clk_data);
+	return mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
 }
 
 static const struct of_device_id mpfs_clk_of_match_table[] = {
diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
index 7f3fb2d472f4..27cd68b4ee81 100644
--- a/drivers/reset/reset-mpfs.c
+++ b/drivers/reset/reset-mpfs.c
@@ -8,6 +8,7 @@
  */
 #include <linux/auxiliary_bus.h>
 #include <linux/delay.h>
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -28,10 +29,11 @@
 /* block concurrent access to the soft reset register */
 static DEFINE_SPINLOCK(mpfs_reset_lock);
 
+static void __iomem *mpfs_reset_addr;
+
 /*
  * Peripheral clock resets
  */
-
 static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
 {
 	unsigned long flags;
@@ -39,9 +41,9 @@ static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
 
 	spin_lock_irqsave(&mpfs_reset_lock, flags);
 
-	reg = mpfs_reset_read(rcdev->dev);
+	reg = readl(mpfs_reset_addr);
 	reg |= BIT(id);
-	mpfs_reset_write(rcdev->dev, reg);
+	writel(reg, mpfs_reset_addr);
 
 	spin_unlock_irqrestore(&mpfs_reset_lock, flags);
 
@@ -55,9 +57,9 @@ static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
 
 	spin_lock_irqsave(&mpfs_reset_lock, flags);
 
-	reg = mpfs_reset_read(rcdev->dev);
+	reg = readl(mpfs_reset_addr);
 	reg &= ~BIT(id);
-	mpfs_reset_write(rcdev->dev, reg);
+	writel(reg, mpfs_reset_addr);
 
 	spin_unlock_irqrestore(&mpfs_reset_lock, flags);
 
@@ -66,7 +68,7 @@ static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
 
 static int mpfs_status(struct reset_controller_dev *rcdev, unsigned long id)
 {
-	u32 reg = mpfs_reset_read(rcdev->dev);
+	u32 reg = readl(mpfs_reset_addr);
 
 	/*
 	 * It is safe to return here as MPFS_NUM_RESETS makes sure the sign bit
@@ -137,9 +139,67 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
 	return devm_reset_controller_register(dev, rcdev);
 }
 
+static void mpfs_reset_unregister_adev(void *_adev)
+{
+	struct auxiliary_device *adev = _adev;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+}
+
+static void mpfs_reset_adev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	kfree(adev);
+}
+
+static struct auxiliary_device *mpfs_reset_adev_alloc(struct device *clk_dev)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return ERR_PTR(-ENOMEM);
+
+	adev->name = "reset-mpfs";
+	adev->dev.parent = clk_dev;
+	adev->dev.release = mpfs_reset_adev_release;
+	adev->id = 666u;
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		kfree(adev);
+		return ERR_PTR(ret);
+	}
+
+	return adev;
+}
+
+int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	mpfs_reset_addr = base;
+
+	adev = mpfs_reset_adev_alloc(clk_dev);
+	if (IS_ERR(adev))
+		return PTR_ERR(adev);
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(clk_dev, mpfs_reset_unregister_adev, adev);
+}
+
 static const struct auxiliary_device_id mpfs_reset_ids[] = {
 	{
-		.name = "clk_mpfs.reset-mpfs",
+		.name = "reset_mpfs.reset-mpfs",
 	},
 	{ }
 };
diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
index 09722f83b0ca..0b756bf5e9bd 100644
--- a/include/soc/microchip/mpfs.h
+++ b/include/soc/microchip/mpfs.h
@@ -43,11 +43,11 @@ struct mtd_info *mpfs_sys_controller_get_flash(struct mpfs_sys_controller *mpfs_
 #endif /* if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL) */
 
 #if IS_ENABLED(CONFIG_MCHP_CLK_MPFS)
-
-u32 mpfs_reset_read(struct device *dev);
-
-void mpfs_reset_write(struct device *dev, u32 val);
-
+#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
+int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base);
+#else
+int mpfs_reset_controller_register(struct device *clk_dev, void* __iomem base) { return 0;}
+#endif /* if IS_ENABLED(CONFIG_RESET_CONTROLLER) */
 #endif /* if IS_ENABLED(CONFIG_MCHP_CLK_MPFS) */
 
 #endif /* __SOC_MPFS_H__ */
Stephen Boyd April 10, 2024, 2:27 a.m. UTC | #7
Quoting Conor Dooley (2024-04-09 05:05:37)
> On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:
> 
> > > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > > > starfive-jh7110 drivers are examples of this.
> > > > 
> > > > > The auxiliary device creation function can also be in the
> > > > > drivers/reset/ directory so that the clk driver calls some function
> > > > > to create and register the device.
> > > > 
> > > > I'm undecided about this, do you think mpfs_reset_controller_register()
> > > > and jh7110_reset_controller_register() should rather live with the
> > > > reset aux drivers in drivers/reset/ ?
> > > 
> > > Yes, and also mpfs_reset_read() and friends. We should pass the base
> > > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> > > then move all that code into drivers/reset with some header file
> > > exported function to call. That way the clk driver hands over the data
> > > without having to implement half the implementation.
> > 
> > I'll todo list that :)
> 
> Something like the below?
> 
> -- >8 --
> From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
> From: Conor Dooley <conor.dooley@microchip.com>
> Date: Tue, 9 Apr 2024 11:54:34 +0100
> Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
>  reset subsystem
> 
> <insert something here>
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Looks pretty good.

>  static const struct of_device_id mpfs_clk_of_match_table[] = {
> diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> index 7f3fb2d472f4..27cd68b4ee81 100644
> --- a/drivers/reset/reset-mpfs.c
> +++ b/drivers/reset/reset-mpfs.c
> @@ -137,9 +139,67 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
>         return devm_reset_controller_register(dev, rcdev);
>  }
>  
> +static void mpfs_reset_unregister_adev(void *_adev)
> +{
> +       struct auxiliary_device *adev = _adev;
> +
> +       auxiliary_device_delete(adev);
> +       auxiliary_device_uninit(adev);
> +}
> +
> +static void mpfs_reset_adev_release(struct device *dev)
> +{
> +       struct auxiliary_device *adev = to_auxiliary_dev(dev);
> +
> +       kfree(adev);
> +}
> +
> +static struct auxiliary_device *mpfs_reset_adev_alloc(struct device *clk_dev)
> +{
> +       struct auxiliary_device *adev;
> +       int ret;
> +
> +       adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> +       if (!adev)
> +               return ERR_PTR(-ENOMEM);
> +
> +       adev->name = "reset-mpfs";
> +       adev->dev.parent = clk_dev;
> +       adev->dev.release = mpfs_reset_adev_release;
> +       adev->id = 666u;
> +
> +       ret = auxiliary_device_init(adev);
> +       if (ret) {
> +               kfree(adev);
> +               return ERR_PTR(ret);
> +       }
> +
> +       return adev;
> +}
> +
> +int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base)
> +{
> +       struct auxiliary_device *adev;
> +       int ret;
> +
> +       mpfs_reset_addr = base;

Instead of a global this can be stashed in adev->dev.platform_data and
grabbed in the driver probe?

> +
> +       adev = mpfs_reset_adev_alloc(clk_dev);
> +       if (IS_ERR(adev))
> +               return PTR_ERR(adev);
> +
> +       ret = auxiliary_device_add(adev);
> +       if (ret) {
> +               auxiliary_device_uninit(adev);
> +               return ret;
> +       }
> +
> +       return devm_add_action_or_reset(clk_dev, mpfs_reset_unregister_adev, adev);
> +}
> +
>  static const struct auxiliary_device_id mpfs_reset_ids[] = {
>         {
> -               .name = "clk_mpfs.reset-mpfs",
> +               .name = "reset_mpfs.reset-mpfs",
>         },
>         { }
>  };
> diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
> index 09722f83b0ca..0b756bf5e9bd 100644
> --- a/include/soc/microchip/mpfs.h
> +++ b/include/soc/microchip/mpfs.h
> @@ -43,11 +43,11 @@ struct mtd_info *mpfs_sys_controller_get_flash(struct mpfs_sys_controller *mpfs_
>  #endif /* if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL) */
>  
>  #if IS_ENABLED(CONFIG_MCHP_CLK_MPFS)
> -
> -u32 mpfs_reset_read(struct device *dev);
> -
> -void mpfs_reset_write(struct device *dev, u32 val);
> -
> +#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
> +int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base);
> +#else
> +int mpfs_reset_controller_register(struct device *clk_dev, void* __iomem base) { return 0;}

static inline
Philipp Zabel April 10, 2024, 8:56 a.m. UTC | #8
On Di, 2024-04-09 at 19:27 -0700, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-04-09 05:05:37)
> > On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:
> > 
> > > > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > > > > starfive-jh7110 drivers are examples of this.
> > > > > 
> > > > > > The auxiliary device creation function can also be in the
> > > > > > drivers/reset/ directory so that the clk driver calls some function
> > > > > > to create and register the device.
> > > > > 
> > > > > I'm undecided about this, do you think mpfs_reset_controller_register()
> > > > > and jh7110_reset_controller_register() should rather live with the
> > > > > reset aux drivers in drivers/reset/ ?
> > > > 
> > > > Yes, and also mpfs_reset_read() and friends. We should pass the base
> > > > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> > > > then move all that code into drivers/reset with some header file
> > > > exported function to call. That way the clk driver hands over the data
> > > > without having to implement half the implementation.
> > > 
> > > I'll todo list that :)
> > 
> > Something like the below?
> > 
> > -- >8 --
> > From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
> > From: Conor Dooley <conor.dooley@microchip.com>
> > Date: Tue, 9 Apr 2024 11:54:34 +0100
> > Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
> >  reset subsystem
> > 
> > <insert something here>
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Looks pretty good.

Yes, that does look convincing.

regards
Philipp
Jerome Brunet April 10, 2024, 9:17 a.m. UTC | #9
On Wed 10 Apr 2024 at 10:56, Philipp Zabel <p.zabel@pengutronix.de> wrote:

> On Di, 2024-04-09 at 19:27 -0700, Stephen Boyd wrote:
>> Quoting Conor Dooley (2024-04-09 05:05:37)
>> > On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:
>> > 
>> > > > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
>> > > > > starfive-jh7110 drivers are examples of this.
>> > > > > 
>> > > > > > The auxiliary device creation function can also be in the
>> > > > > > drivers/reset/ directory so that the clk driver calls some function
>> > > > > > to create and register the device.
>> > > > > 
>> > > > > I'm undecided about this, do you think mpfs_reset_controller_register()
>> > > > > and jh7110_reset_controller_register() should rather live with the
>> > > > > reset aux drivers in drivers/reset/ ?
>> > > > 
>> > > > Yes, and also mpfs_reset_read() and friends. We should pass the base
>> > > > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
>> > > > then move all that code into drivers/reset with some header file
>> > > > exported function to call. That way the clk driver hands over the data
>> > > > without having to implement half the implementation.
>> > > 
>> > > I'll todo list that :)
>> > 
>> > Something like the below?
>> > 
>> > -- >8 --
>> > From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
>> > From: Conor Dooley <conor.dooley@microchip.com>
>> > Date: Tue, 9 Apr 2024 11:54:34 +0100
>> > Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
>> >  reset subsystem
>> > 
>> > <insert something here>
>> > 
>> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> 
>> Looks pretty good.
>
> Yes, that does look convincing.

Thanks a lot for the example Conor !

When it comes to Amlogic, I think the bits of reset instanciated by
clock drivers could probably share (part of) the code of the existing
amlogic reset driver. Some have toggle only, no level, but it is mostly
the same.

I'll try to have a look at it

>
> regards
> Philipp
Conor Dooley April 10, 2024, 9:50 a.m. UTC | #10
On Tue, Apr 09, 2024 at 07:27:36PM -0700, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-04-09 05:05:37)
> > On Mon, Apr 08, 2024 at 06:05:51PM +0100, Conor Dooley wrote:
> > 
> > > > > Seconded, the clk-mpfs/reset-mpfs and clk-starfive-jh7110-sys/reset-
> > > > > starfive-jh7110 drivers are examples of this.
> > > > > 
> > > > > > The auxiliary device creation function can also be in the
> > > > > > drivers/reset/ directory so that the clk driver calls some function
> > > > > > to create and register the device.
> > > > > 
> > > > > I'm undecided about this, do you think mpfs_reset_controller_register()
> > > > > and jh7110_reset_controller_register() should rather live with the
> > > > > reset aux drivers in drivers/reset/ ?
> > > > 
> > > > Yes, and also mpfs_reset_read() and friends. We should pass the base
> > > > iomem pointer and parent device to mpfs_reset_adev_alloc() instead and
> > > > then move all that code into drivers/reset with some header file
> > > > exported function to call. That way the clk driver hands over the data
> > > > without having to implement half the implementation.
> > > 
> > > I'll todo list that :)
> > 
> > Something like the below?
> > 
> > -- >8 --
> > From a12f281d2cb869bcd9a6ffc45d0c6a0d3aa2e9e2 Mon Sep 17 00:00:00 2001
> > From: Conor Dooley <conor.dooley@microchip.com>
> > Date: Tue, 9 Apr 2024 11:54:34 +0100
> > Subject: [PATCH] clock, reset: microchip: move all mpfs reset code to the
> >  reset subsystem
> > 
> > <insert something here>
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Looks pretty good.
> 
> >  static const struct of_device_id mpfs_clk_of_match_table[] = {
> > diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> > index 7f3fb2d472f4..27cd68b4ee81 100644
> > --- a/drivers/reset/reset-mpfs.c
> > +++ b/drivers/reset/reset-mpfs.c
> > @@ -137,9 +139,67 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
> >         return devm_reset_controller_register(dev, rcdev);
> >  }
> >  
> > +static void mpfs_reset_unregister_adev(void *_adev)
> > +{
> > +       struct auxiliary_device *adev = _adev;
> > +
> > +       auxiliary_device_delete(adev);
> > +       auxiliary_device_uninit(adev);
> > +}
> > +
> > +static void mpfs_reset_adev_release(struct device *dev)
> > +{
> > +       struct auxiliary_device *adev = to_auxiliary_dev(dev);
> > +
> > +       kfree(adev);
> > +}
> > +
> > +static struct auxiliary_device *mpfs_reset_adev_alloc(struct device *clk_dev)
> > +{
> > +       struct auxiliary_device *adev;
> > +       int ret;
> > +
> > +       adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> > +       if (!adev)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       adev->name = "reset-mpfs";
> > +       adev->dev.parent = clk_dev;
> > +       adev->dev.release = mpfs_reset_adev_release;
> > +       adev->id = 666u;
> > +
> > +       ret = auxiliary_device_init(adev);
> > +       if (ret) {
> > +               kfree(adev);
> > +               return ERR_PTR(ret);
> > +       }
> > +
> > +       return adev;
> > +}
> > +
> > +int mpfs_reset_controller_register(struct device *clk_dev, void __iomem *base)
> > +{
> > +       struct auxiliary_device *adev;
> > +       int ret;
> > +
> > +       mpfs_reset_addr = base;
> 
> Instead of a global this can be stashed in adev->dev.platform_data and
> grabbed in the driver probe?

I suppose, really I was just being "lazy" here and creating a global
rather than a `struct mpfs_reset` containing only the base address.

The test robot reported some issues with hexagon & COMPILE_TEST, so I'll
push it out again with this change and the fixes for the reported issues
and send the patch ones it gets the all clear.

Cheers,
Conor.
diff mbox series

Patch

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 29ffd14d267b..d6a2fa5f7e88 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -48,6 +48,10 @@  config COMMON_CLK_MESON_CPU_DYNDIV
 	tristate
 	select COMMON_CLK_MESON_REGMAP
 
+config COMMON_CLK_MESON_AUDIO_RSTC
+	tristate
+	select RESET_CONTROLLER
+
 config COMMON_CLK_MESON8B
 	bool "Meson8 SoC Clock controller support"
 	depends on ARM
@@ -101,6 +105,7 @@  config COMMON_CLK_AXG_AUDIO
 	select COMMON_CLK_MESON_PHASE
 	select COMMON_CLK_MESON_SCLK_DIV
 	select COMMON_CLK_MESON_CLKC_UTILS
+	select COMMON_CLK_MESON_AUDIO_RSTC
 	select REGMAP_MMIO
 	help
 	  Support for the audio clock controller on AmLogic A113D devices,
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 9ee4b954c896..88d94921a4dc 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -12,6 +12,7 @@  obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
 obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
 obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
 obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
+obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
 
 # Amlogic Clock controllers
 
diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
index ac3482960903..990203a7ad5c 100644
--- a/drivers/clk/meson/axg-audio.c
+++ b/drivers/clk/meson/axg-audio.c
@@ -12,10 +12,10 @@ 
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
-#include <linux/reset-controller.h>
 #include <linux/slab.h>
 
 #include "meson-clkc-utils.h"
+#include "meson-audio-rstc.h"
 #include "axg-audio.h"
 #include "clk-regmap.h"
 #include "clk-phase.h"
@@ -1648,84 +1648,6 @@  static struct clk_regmap *const sm1_clk_regmaps[] = {
 	&sm1_sysclk_b_en,
 };
 
-struct axg_audio_reset_data {
-	struct reset_controller_dev rstc;
-	struct regmap *map;
-	unsigned int offset;
-};
-
-static void axg_audio_reset_reg_and_bit(struct axg_audio_reset_data *rst,
-					unsigned long id,
-					unsigned int *reg,
-					unsigned int *bit)
-{
-	unsigned int stride = regmap_get_reg_stride(rst->map);
-
-	*reg = (id / (stride * BITS_PER_BYTE)) * stride;
-	*reg += rst->offset;
-	*bit = id % (stride * BITS_PER_BYTE);
-}
-
-static int axg_audio_reset_update(struct reset_controller_dev *rcdev,
-				unsigned long id, bool assert)
-{
-	struct axg_audio_reset_data *rst =
-		container_of(rcdev, struct axg_audio_reset_data, rstc);
-	unsigned int offset, bit;
-
-	axg_audio_reset_reg_and_bit(rst, id, &offset, &bit);
-
-	regmap_update_bits(rst->map, offset, BIT(bit),
-			assert ? BIT(bit) : 0);
-
-	return 0;
-}
-
-static int axg_audio_reset_status(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	struct axg_audio_reset_data *rst =
-		container_of(rcdev, struct axg_audio_reset_data, rstc);
-	unsigned int val, offset, bit;
-
-	axg_audio_reset_reg_and_bit(rst, id, &offset, &bit);
-
-	regmap_read(rst->map, offset, &val);
-
-	return !!(val & BIT(bit));
-}
-
-static int axg_audio_reset_assert(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	return axg_audio_reset_update(rcdev, id, true);
-}
-
-static int axg_audio_reset_deassert(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	return axg_audio_reset_update(rcdev, id, false);
-}
-
-static int axg_audio_reset_toggle(struct reset_controller_dev *rcdev,
-				unsigned long id)
-{
-	int ret;
-
-	ret = axg_audio_reset_assert(rcdev, id);
-	if (ret)
-		return ret;
-
-	return axg_audio_reset_deassert(rcdev, id);
-}
-
-static const struct reset_control_ops axg_audio_rstc_ops = {
-	.assert = axg_audio_reset_assert,
-	.deassert = axg_audio_reset_deassert,
-	.reset = axg_audio_reset_toggle,
-	.status = axg_audio_reset_status,
-};
-
 static const struct regmap_config axg_audio_regmap_cfg = {
 	.reg_bits	= 32,
 	.val_bits	= 32,
@@ -1745,7 +1667,6 @@  static int axg_audio_clkc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	const struct audioclk_data *data;
-	struct axg_audio_reset_data *rst;
 	struct regmap *map;
 	void __iomem *regs;
 	struct clk_hw *hw;
@@ -1807,18 +1728,8 @@  static int axg_audio_clkc_probe(struct platform_device *pdev)
 	if (!data->reset_num)
 		return 0;
 
-	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
-	if (!rst)
-		return -ENOMEM;
-
-	rst->map = map;
-	rst->offset = data->reset_offset;
-	rst->rstc.nr_resets = data->reset_num;
-	rst->rstc.ops = &axg_audio_rstc_ops;
-	rst->rstc.of_node = dev->of_node;
-	rst->rstc.owner = THIS_MODULE;
-
-	return devm_reset_controller_register(dev, &rst->rstc);
+	return meson_audio_rstc_register(dev, map, data->reset_offset,
+					 data->reset_num);
 }
 
 static const struct audioclk_data axg_audioclk_data = {
diff --git a/drivers/clk/meson/meson-audio-rstc.c b/drivers/clk/meson/meson-audio-rstc.c
new file mode 100644
index 000000000000..2079d24c40f4
--- /dev/null
+++ b/drivers/clk/meson/meson-audio-rstc.c
@@ -0,0 +1,109 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (c) 2018 BayLibre, SAS.
+ * Author: Jerome Brunet <jbrunet@baylibre.com>
+ */
+
+#include <linux/reset-controller.h>
+
+#include "meson-audio-rstc.h"
+
+struct meson_audio_reset_data {
+	struct reset_controller_dev rstc;
+	struct regmap *map;
+	unsigned int offset;
+};
+
+static void meson_audio_reset_reg_and_bit(struct meson_audio_reset_data *rst,
+					  unsigned long id,
+					  unsigned int *reg,
+					  unsigned int *bit)
+{
+	unsigned int stride = regmap_get_reg_stride(rst->map);
+
+	*reg = (id / (stride * BITS_PER_BYTE)) * stride;
+	*reg += rst->offset;
+	*bit = id % (stride * BITS_PER_BYTE);
+}
+
+static int meson_audio_reset_update(struct reset_controller_dev *rcdev,
+				    unsigned long id, bool assert)
+{
+	struct meson_audio_reset_data *rst =
+		container_of(rcdev, struct meson_audio_reset_data, rstc);
+	unsigned int offset, bit;
+
+	meson_audio_reset_reg_and_bit(rst, id, &offset, &bit);
+
+	regmap_update_bits(rst->map, offset, BIT(bit),
+			assert ? BIT(bit) : 0);
+
+	return 0;
+}
+
+static int meson_audio_reset_status(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct meson_audio_reset_data *rst =
+		container_of(rcdev, struct meson_audio_reset_data, rstc);
+	unsigned int val, offset, bit;
+
+	meson_audio_reset_reg_and_bit(rst, id, &offset, &bit);
+
+	regmap_read(rst->map, offset, &val);
+
+	return !!(val & BIT(bit));
+}
+
+static int meson_audio_reset_assert(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	return meson_audio_reset_update(rcdev, id, true);
+}
+
+static int meson_audio_reset_deassert(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	return meson_audio_reset_update(rcdev, id, false);
+}
+
+static int meson_audio_reset_toggle(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	int ret;
+
+	ret = meson_audio_reset_assert(rcdev, id);
+	if (ret)
+		return ret;
+
+	return meson_audio_reset_deassert(rcdev, id);
+}
+
+static const struct reset_control_ops meson_audio_rstc_ops = {
+	.assert = meson_audio_reset_assert,
+	.deassert = meson_audio_reset_deassert,
+	.reset = meson_audio_reset_toggle,
+	.status = meson_audio_reset_status,
+};
+
+int meson_audio_rstc_register(struct device *dev, struct regmap *map,
+			      unsigned int offset, unsigned int num)
+{
+	struct meson_audio_reset_data *rst;
+
+	rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
+	if (!rst)
+		return -ENOMEM;
+
+	rst->map = map;
+	rst->offset = offset;
+	rst->rstc.nr_resets = num;
+	rst->rstc.ops = &meson_audio_rstc_ops;
+	rst->rstc.of_node = dev->of_node;
+	rst->rstc.owner = THIS_MODULE;
+
+	return devm_reset_controller_register(dev, &rst->rstc);
+}
+EXPORT_SYMBOL_GPL(meson_audio_rstc_register);
+
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/meson/meson-audio-rstc.h b/drivers/clk/meson/meson-audio-rstc.h
new file mode 100644
index 000000000000..6b441549de03
--- /dev/null
+++ b/drivers/clk/meson/meson-audio-rstc.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+
+#ifndef __MESON_AUDIO_RSTC_H
+#define __MESON_AUDIO_RSTC_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+int meson_audio_rstc_register(struct device *dev, struct regmap *map,
+			      unsigned int offset, unsigned int num);
+
+#endif /* __MESON_AUDIO_RSTC_H */