[PATCH 0/2] reset: amlogic: rework auxiliary reset support

[PATCH 0/2] reset: amlogic: rework auxiliary reset support

[Jerome Brunet]

The current implementation of auxiliary reset controller, with the
registration helper in the same module as the driver code, does not
allow to properly decouple the registered auxiliary driver from
the registering one.

This patchset removes the registration helper from the auxiliary reset
driver and changes how regmap is passed, to simplify the interface a bit.

This patcheset depends on:
commit 5ae1a43486fb ("clk: amlogic: axg-audio: revert reset implementation")

The above removes the only user of the auxiliary reset controller, restoring
old implementation as a temporary solution, while it is reworked.

The commit has been applied to clock fixes [1] branch and will eventually
make its way to an rc release.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-fixes&id=5ae1a43486fb3febd5ce78da13eb354f16d049e0

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Jerome Brunet (2):
      reset: amlogic: aux: get regmap through parent device
      reset: amlogic: aux: drop aux registration helper

 drivers/reset/amlogic/reset-meson-aux.c | 85 +++------------------------------
 include/soc/amlogic/reset-meson-aux.h   | 23 ---------
 2 files changed, 6 insertions(+), 102 deletions(-)
---
base-commit: 3d99f9231bedcf9acfb965a97645a8ecfa93a40d
change-id: 20241209-meson-rst-aux-rework-e26c716c6762
prerequisite-change-id: 20241127-clk-audio-fix-rst-missing-0b80628d934b:v2
prerequisite-patch-id: 8bf55ab8ba9db1abea5df2554864a2f4f9c72e77

Best regards,
-- 
Jerome

[PATCH 1/2] reset: amlogic: aux: get regmap through parent device

[Jerome Brunet]

Get regmap directly from the parent device registering the
auxiliary reset driver, instead of using device data attached
to the auxiliary device.

This simplifies the registration a bit.

Suggested-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/reset/amlogic/reset-meson-aux.c | 36 +++++++++++----------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/reset/amlogic/reset-meson-aux.c b/drivers/reset/amlogic/reset-meson-aux.c
index 4b422ae5fcd209e1041e9a793d980627203d00b0..0fc1788eb7a83810ac524facfe09b5fa032573c3 100644
--- a/drivers/reset/amlogic/reset-meson-aux.c
+++ b/drivers/reset/amlogic/reset-meson-aux.c
@@ -18,14 +18,6 @@
 
 static DEFINE_IDA(meson_rst_aux_ida);
 
-struct meson_reset_adev {
-	struct auxiliary_device adev;
-	struct regmap *map;
-};
-
-#define to_meson_reset_adev(_adev) \
-	container_of((_adev), struct meson_reset_adev, adev)
-
 static const struct meson_reset_param meson_a1_audio_param = {
 	.reset_ops	= &meson_reset_toggle_ops,
 	.reset_num	= 32,
@@ -72,10 +64,13 @@ static int meson_reset_aux_probe(struct auxiliary_device *adev,
 {
 	const struct meson_reset_param *param =
 		(const struct meson_reset_param *)(id->driver_data);
-	struct meson_reset_adev *raux =
-		to_meson_reset_adev(adev);
+	struct regmap *map;
+
+	map = dev_get_regmap(adev->dev.parent, NULL);
+	if (!map)
+		return -EINVAL;
 
-	return meson_reset_controller_register(&adev->dev, raux->map, param);
+	return meson_reset_controller_register(&adev->dev, map, param);
 }
 
 static struct auxiliary_driver meson_reset_aux_driver = {
@@ -87,11 +82,9 @@ module_auxiliary_driver(meson_reset_aux_driver);
 static void meson_rst_aux_release(struct device *dev)
 {
 	struct auxiliary_device *adev = to_auxiliary_dev(dev);
-	struct meson_reset_adev *raux =
-		to_meson_reset_adev(adev);
 
 	ida_free(&meson_rst_aux_ida, adev->id);
-	kfree(raux);
+	kfree(adev);
 }
 
 static void meson_rst_aux_unregister_adev(void *_adev)
@@ -103,24 +96,19 @@ static void meson_rst_aux_unregister_adev(void *_adev)
 }
 
 int devm_meson_rst_aux_register(struct device *dev,
-				struct regmap *map,
 				const char *adev_name)
 {
-	struct meson_reset_adev *raux;
 	struct auxiliary_device *adev;
 	int ret;
 
-	raux = kzalloc(sizeof(*raux), GFP_KERNEL);
-	if (!raux)
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
 		return -ENOMEM;
 
 	ret = ida_alloc(&meson_rst_aux_ida, GFP_KERNEL);
 	if (ret < 0)
-		goto raux_free;
-
-	raux->map = map;
+		goto adev_free;
 
-	adev = &raux->adev;
 	adev->id = ret;
 	adev->name = adev_name;
 	adev->dev.parent = dev;
@@ -142,8 +130,8 @@ int devm_meson_rst_aux_register(struct device *dev,
 
 ida_free:
 	ida_free(&meson_rst_aux_ida, adev->id);
-raux_free:
-	kfree(raux);
+adev_free:
+	kfree(adev);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(devm_meson_rst_aux_register);

-- 
2.45.2

[PATCH 2/2] reset: amlogic: aux: drop aux registration helper

[Jerome Brunet]

Having the aux registration helper along with the registered driver is not
great dependency wise. It does not allow the registering driver to be
properly decoupled from the registered auxiliary driver.

Drop the registration helper from the amlogic auxiliary reset driver.
This will be handled in the registering clock driver to start with while
a more generic solution is worked on.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/reset/amlogic/reset-meson-aux.c | 61 ---------------------------------
 include/soc/amlogic/reset-meson-aux.h   | 23 -------------
 2 files changed, 84 deletions(-)

diff --git a/drivers/reset/amlogic/reset-meson-aux.c b/drivers/reset/amlogic/reset-meson-aux.c
index 0fc1788eb7a83810ac524facfe09b5fa032573c3..1a2024afa056ea4d9404f6444476e48265ac8da4 100644
--- a/drivers/reset/amlogic/reset-meson-aux.c
+++ b/drivers/reset/amlogic/reset-meson-aux.c
@@ -11,12 +11,8 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
-#include <linux/slab.h>
 
 #include "reset-meson.h"
-#include <soc/amlogic/reset-meson-aux.h>
-
-static DEFINE_IDA(meson_rst_aux_ida);
 
 static const struct meson_reset_param meson_a1_audio_param = {
 	.reset_ops	= &meson_reset_toggle_ops,
@@ -79,63 +75,6 @@ static struct auxiliary_driver meson_reset_aux_driver = {
 };
 module_auxiliary_driver(meson_reset_aux_driver);
 
-static void meson_rst_aux_release(struct device *dev)
-{
-	struct auxiliary_device *adev = to_auxiliary_dev(dev);
-
-	ida_free(&meson_rst_aux_ida, adev->id);
-	kfree(adev);
-}
-
-static void meson_rst_aux_unregister_adev(void *_adev)
-{
-	struct auxiliary_device *adev = _adev;
-
-	auxiliary_device_delete(adev);
-	auxiliary_device_uninit(adev);
-}
-
-int devm_meson_rst_aux_register(struct device *dev,
-				const char *adev_name)
-{
-	struct auxiliary_device *adev;
-	int ret;
-
-	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
-	if (!adev)
-		return -ENOMEM;
-
-	ret = ida_alloc(&meson_rst_aux_ida, GFP_KERNEL);
-	if (ret < 0)
-		goto adev_free;
-
-	adev->id = ret;
-	adev->name = adev_name;
-	adev->dev.parent = dev;
-	adev->dev.release = meson_rst_aux_release;
-	device_set_of_node_from_dev(&adev->dev, dev);
-
-	ret = auxiliary_device_init(adev);
-	if (ret)
-		goto ida_free;
-
-	ret = __auxiliary_device_add(adev, dev->driver->name);
-	if (ret) {
-		auxiliary_device_uninit(adev);
-		return ret;
-	}
-
-	return devm_add_action_or_reset(dev, meson_rst_aux_unregister_adev,
-					adev);
-
-ida_free:
-	ida_free(&meson_rst_aux_ida, adev->id);
-adev_free:
-	kfree(adev);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(devm_meson_rst_aux_register);
-
 MODULE_DESCRIPTION("Amlogic Meson Reset Auxiliary driver");
 MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
 MODULE_LICENSE("Dual BSD/GPL");
diff --git a/include/soc/amlogic/reset-meson-aux.h b/include/soc/amlogic/reset-meson-aux.h
deleted file mode 100644
index d8a15d48c98494c45c6e3d7e88fa714f770f12e3..0000000000000000000000000000000000000000
--- a/include/soc/amlogic/reset-meson-aux.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __SOC_RESET_MESON_AUX_H
-#define __SOC_RESET_MESON_AUX_H
-
-#include <linux/err.h>
-
-struct device;
-struct regmap;
-
-#if IS_ENABLED(CONFIG_RESET_MESON_AUX)
-int devm_meson_rst_aux_register(struct device *dev,
-				struct regmap *map,
-				const char *adev_name);
-#else
-static inline int devm_meson_rst_aux_register(struct device *dev,
-					      struct regmap *map,
-					      const char *adev_name)
-{
-	return 0;
-}
-#endif
-
-#endif /* __SOC_RESET_MESON_AUX_H */

-- 
2.45.2

Re: [PATCH 0/2] reset: amlogic: rework auxiliary reset support

[Arnd Bergmann]

On Mon, Dec 9, 2024, at 17:04, Jerome Brunet wrote:
> The current implementation of auxiliary reset controller, with the
> registration helper in the same module as the driver code, does not
> allow to properly decouple the registered auxiliary driver from
> the registering one.
>
> This patchset removes the registration helper from the auxiliary reset
> driver and changes how regmap is passed, to simplify the interface a bit.
>

Looks good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 0/2] reset: amlogic: rework auxiliary reset support

[Jerome Brunet]

On Mon 09 Dec 2024 at 17:04, Jerome Brunet <jbrunet@baylibre.com> wrote:

> The current implementation of auxiliary reset controller, with the
> registration helper in the same module as the driver code, does not
> allow to properly decouple the registered auxiliary driver from
> the registering one.
>
> This patchset removes the registration helper from the auxiliary reset
> driver and changes how regmap is passed, to simplify the interface a bit.
>
> This patcheset depends on:
> commit 5ae1a43486fb ("clk: amlogic: axg-audio: revert reset implementation")
>
> The above removes the only user of the auxiliary reset controller, restoring
> old implementation as a temporary solution, while it is reworked.
>
> The commit has been applied to clock fixes [1] branch and will eventually
> make its way to an rc release.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-fixes&id=5ae1a43486fb3febd5ce78da13eb354f16d049e0

Hi Philipp,

FYI, the dependency above has made it to v6.13-rc3

Cheers

>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
> Jerome Brunet (2):
>       reset: amlogic: aux: get regmap through parent device
>       reset: amlogic: aux: drop aux registration helper
>
>  drivers/reset/amlogic/reset-meson-aux.c | 85 +++------------------------------
>  include/soc/amlogic/reset-meson-aux.h   | 23 ---------
>  2 files changed, 6 insertions(+), 102 deletions(-)
> ---
> base-commit: 3d99f9231bedcf9acfb965a97645a8ecfa93a40d
> change-id: 20241209-meson-rst-aux-rework-e26c716c6762
> prerequisite-change-id: 20241127-clk-audio-fix-rst-missing-0b80628d934b:v2
> prerequisite-patch-id: 8bf55ab8ba9db1abea5df2554864a2f4f9c72e77
>
> Best regards,

-- 
Jerome