2024-05-16 15:09:02

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 0/9] reset: amlogic: move reset drivers out of CCF

This RFC follows the discussion about having reset driver in the clock tree
[1]. Ideally those should reside in the reset part of tree.

Also the code of the amlogic reset driver is very similar between the 2 trees
and could use the same driver code.

This RFC moves the reset driver of audio clock controller of the g12 and
sm1 SoC family to the reset tree, using the auxiliary bus.

The infrastructure put in place is meant to be generic enough so we may
eventually also move the reset drivers in the meson8b and aoclk clock
controllers.

[1] https://lore.kernel.org/linux-clk/[email protected]

Jerome Brunet (9):
reset: amlogic: convert driver to regmap
reset: amlogic: add driver parameters
reset: amlogic: split the device and platform probe
reset: amlogic: use reset number instead of register count
reset: amlogic: add reset status support
reset: amlogic: add toggle reset support
reset: amlogic: add auxiliary reset driver support
clk: meson: add auxiliary reset helper driver
clk: amlogic: axg-audio: use the auxiliary reset driver

drivers/clk/meson/Kconfig | 6 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/axg-audio.c | 108 +--------
drivers/clk/meson/meson-clk-rst-aux.c | 84 +++++++
drivers/clk/meson/meson-clk-rst-aux.h | 14 ++
drivers/reset/Kconfig | 1 +
drivers/reset/reset-meson.c | 210 ++++++++++++++----
include/soc/amlogic/meson8b-auxiliary-reset.h | 17 ++
8 files changed, 293 insertions(+), 148 deletions(-)
create mode 100644 drivers/clk/meson/meson-clk-rst-aux.c
create mode 100644 drivers/clk/meson/meson-clk-rst-aux.h
create mode 100644 include/soc/amlogic/meson8b-auxiliary-reset.h

--
2.43.0



2024-05-16 15:09:07

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 1/9] reset: amlogic: convert driver to regmap

To allow using the same driver for the main reset controller and the
auxiliary ones embedded in the clock controllers, convert the
the Amlogic reset driver to regmap.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 80 ++++++++++++++++++++-----------------
1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index f78be97898bc..8f3d6e9df235 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -11,36 +11,44 @@
#include <linux/of.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/regmap.h>
#include <linux/reset-controller.h>
#include <linux/slab.h>
#include <linux/types.h>

-#define BITS_PER_REG 32
-
struct meson_reset_param {
int reg_count;
int level_offset;
};

struct meson_reset {
- void __iomem *reg_base;
const struct meson_reset_param *param;
struct reset_controller_dev rcdev;
- spinlock_t lock;
+ struct regmap *map;
};

+static void meson_reset_offset_and_bit(struct meson_reset *data,
+ unsigned long id,
+ unsigned int *offset,
+ unsigned int *bit)
+{
+ unsigned int stride = regmap_get_reg_stride(data->map);
+
+ *offset = (id / (stride * BITS_PER_BYTE)) * stride;
+ *bit = id % (stride * BITS_PER_BYTE);
+}
+
static int meson_reset_reset(struct reset_controller_dev *rcdev,
- unsigned long id)
+ unsigned long id)
{
struct meson_reset *data =
container_of(rcdev, struct meson_reset, rcdev);
- unsigned int bank = id / BITS_PER_REG;
- unsigned int offset = id % BITS_PER_REG;
- void __iomem *reg_addr = data->reg_base + (bank << 2);
+ unsigned int offset, bit;

- writel(BIT(offset), reg_addr);
+ meson_reset_offset_and_bit(data, id, &offset, &bit);

- return 0;
+ return regmap_update_bits(data->map, offset,
+ BIT(bit), BIT(bit));
}

static int meson_reset_level(struct reset_controller_dev *rcdev,
@@ -48,25 +56,13 @@ static int meson_reset_level(struct reset_controller_dev *rcdev,
{
struct meson_reset *data =
container_of(rcdev, struct meson_reset, rcdev);
- unsigned int bank = id / BITS_PER_REG;
- unsigned int offset = id % BITS_PER_REG;
- void __iomem *reg_addr;
- unsigned long flags;
- u32 reg;
+ unsigned int offset, bit;

- reg_addr = data->reg_base + data->param->level_offset + (bank << 2);
+ meson_reset_offset_and_bit(data, id, &offset, &bit);
+ offset += data->param->level_offset;

- spin_lock_irqsave(&data->lock, flags);
-
- reg = readl(reg_addr);
- if (assert)
- writel(reg & ~BIT(offset), reg_addr);
- else
- writel(reg | BIT(offset), reg_addr);
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
+ return regmap_update_bits(data->map, offset,
+ BIT(bit), assert ? 0 : BIT(bit));
}

static int meson_reset_assert(struct reset_controller_dev *rcdev,
@@ -113,30 +109,42 @@ static const struct of_device_id meson_reset_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, meson_reset_dt_ids);

+static const struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
static int meson_reset_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct meson_reset *data;
+ void __iomem *base;

- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;

- data->reg_base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(data->reg_base))
- return PTR_ERR(data->reg_base);
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);

- data->param = of_device_get_match_data(&pdev->dev);
+ data->param = of_device_get_match_data(dev);
if (!data->param)
return -ENODEV;

- spin_lock_init(&data->lock);
+ data->map = devm_regmap_init_mmio(dev, base, &regmap_config);
+ if (IS_ERR(data->map))
+ return dev_err_probe(dev, PTR_ERR(data->map),
+ "can't init regmap mmio region\n");

data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = data->param->reg_count * BITS_PER_REG;
+ data->rcdev.nr_resets = data->param->reg_count * BITS_PER_BYTE
+ * regmap_config.reg_stride;
data->rcdev.ops = &meson_reset_ops;
- data->rcdev.of_node = pdev->dev.of_node;
+ data->rcdev.of_node = dev->of_node;

- return devm_reset_controller_register(&pdev->dev, &data->rcdev);
+ return devm_reset_controller_register(dev, &data->rcdev);
}

static struct platform_driver meson_reset_driver = {
--
2.43.0


2024-05-16 15:09:20

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 2/9] reset: amlogic: add driver parameters

To allow using the same driver for the main reset controller and the
auxiliary ones embedded in the clock controllers, allow to customise
the reset offset, same as the level offset. Also add an option to make
the level reset active low or high.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index 8f3d6e9df235..59126c9f194a 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -18,7 +18,9 @@

struct meson_reset_param {
int reg_count;
+ int reset_offset;
int level_offset;
+ bool level_low_reset;
};

struct meson_reset {
@@ -46,6 +48,7 @@ static int meson_reset_reset(struct reset_controller_dev *rcdev,
unsigned int offset, bit;

meson_reset_offset_and_bit(data, id, &offset, &bit);
+ offset += data->param->reset_offset;

return regmap_update_bits(data->map, offset,
BIT(bit), BIT(bit));
@@ -60,9 +63,10 @@ static int meson_reset_level(struct reset_controller_dev *rcdev,

meson_reset_offset_and_bit(data, id, &offset, &bit);
offset += data->param->level_offset;
+ assert ^= data->param->level_low_reset;

return regmap_update_bits(data->map, offset,
- BIT(bit), assert ? 0 : BIT(bit));
+ BIT(bit), assert ? BIT(bit) : 0);
}

static int meson_reset_assert(struct reset_controller_dev *rcdev,
@@ -85,17 +89,23 @@ static const struct reset_control_ops meson_reset_ops = {

static const struct meson_reset_param meson8b_param = {
.reg_count = 8,
+ .reset_offset = 0x0,
.level_offset = 0x7c,
+ .level_low_reset = true,
};

static const struct meson_reset_param meson_a1_param = {
.reg_count = 3,
+ .reset_offset = 0x0,
.level_offset = 0x40,
+ .level_low_reset = true,
};

static const struct meson_reset_param meson_s4_param = {
.reg_count = 6,
+ .reset_offset = 0x0,
.level_offset = 0x40,
+ .level_low_reset = true,
};

static const struct of_device_id meson_reset_dt_ids[] = {
--
2.43.0


2024-05-16 15:09:48

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 4/9] reset: amlogic: use reset number instead of register count

The reset driver from audio clock controller may register less
reset than a register can hold. To avoid making any change while
switching to auxiliary support, use the number of reset instead of the
register count to define the bounds of the reset controller.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index fec55321b52b..3e0447366ba6 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -17,7 +17,7 @@
#include <linux/types.h>

struct meson_reset_param {
- int reg_count;
+ unsigned int reset_num;
int reset_offset;
int level_offset;
bool level_low_reset;
@@ -90,7 +90,6 @@ static const struct reset_control_ops meson_reset_ops = {
static int meson_reset_probe(struct device *dev, struct regmap *map,
const struct meson_reset_param *param)
{
- unsigned int stride = regmap_get_reg_stride(map);
struct meson_reset *data;

data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -100,8 +99,7 @@ static int meson_reset_probe(struct device *dev, struct regmap *map,
data->param = param;
data->map = map;
data->rcdev.owner = dev->driver->owner;
- data->rcdev.nr_resets = param->reg_count * BITS_PER_BYTE
- * stride;
+ data->rcdev.nr_resets = param->reset_num;
data->rcdev.ops = &meson_reset_ops;
data->rcdev.of_node = dev->of_node;

@@ -109,21 +107,21 @@ static int meson_reset_probe(struct device *dev, struct regmap *map,
}

static const struct meson_reset_param meson8b_param = {
- .reg_count = 8,
+ .reset_num = 256,
.reset_offset = 0x0,
.level_offset = 0x7c,
.level_low_reset = true,
};

static const struct meson_reset_param meson_a1_param = {
- .reg_count = 3,
+ .reset_num = 96,
.reset_offset = 0x0,
.level_offset = 0x40,
.level_low_reset = true,
};

static const struct meson_reset_param meson_s4_param = {
- .reg_count = 6,
+ .reset_num = 192,
.reset_offset = 0x0,
.level_offset = 0x40,
.level_low_reset = true,
--
2.43.0


2024-05-16 15:10:03

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 5/9] reset: amlogic: add reset status support

Add the callback to check the status of the level reset, as done in
the reset driver of the audio clock controller.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index 3e0447366ba6..65ba9190cb53 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -69,6 +69,23 @@ static int meson_reset_level(struct reset_controller_dev *rcdev,
BIT(bit), assert ? BIT(bit) : 0);
}

+static int meson_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct meson_reset *data =
+ container_of(rcdev, struct meson_reset, rcdev);
+ unsigned int val, offset, bit;
+
+ meson_reset_offset_and_bit(data, id, &offset, &bit);
+ offset += data->param->level_offset;
+
+ regmap_read(data->map, offset, &val);
+ val = !!(BIT(bit) & val);
+
+
+ return val ^ data->param->level_low_reset;
+}
+
static int meson_reset_assert(struct reset_controller_dev *rcdev,
unsigned long id)
{
@@ -85,6 +102,7 @@ static const struct reset_control_ops meson_reset_ops = {
.reset = meson_reset_reset,
.assert = meson_reset_assert,
.deassert = meson_reset_deassert,
+ .status = meson_reset_status,
};

static int meson_reset_probe(struct device *dev, struct regmap *map,
--
2.43.0


2024-05-16 15:10:42

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 9/9] clk: amlogic: axg-audio: use the auxiliary reset driver

Remove the implementation of the reset driver in axg audio
clock driver and migrate to the one provided by reset framework
on auxiliary bus

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/clk/meson/Kconfig | 1 +
drivers/clk/meson/axg-audio.c | 108 +++-------------------------------
2 files changed, 9 insertions(+), 100 deletions(-)

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 6905aa2f080c..b89e769a6362 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -110,6 +110,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_CLK_RST_AUX
select REGMAP_MMIO
help
Support for the audio clock controller on AmLogic A113D devices,
diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
index 93e6a6d321be..dc9191a9ab0f 100644
--- a/drivers/clk/meson/axg-audio.c
+++ b/drivers/clk/meson/axg-audio.c
@@ -15,6 +15,7 @@
#include <linux/reset-controller.h>
#include <linux/slab.h>

+#include "meson-clk-rst-aux.h"
#include "meson-clkc-utils.h"
#include "axg-audio.h"
#include "clk-regmap.h"
@@ -1678,84 +1679,6 @@ static struct clk_regmap *const sm1_clk_regmaps[] = {
&sm1_earcrx_dmac_clk,
};

-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,
@@ -1766,15 +1689,13 @@ struct audioclk_data {
struct clk_regmap *const *regmap_clks;
unsigned int regmap_clk_num;
struct meson_clk_hw_data hw_clks;
- unsigned int reset_offset;
- unsigned int reset_num;
+ const char *rst_drvname;
};

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;
@@ -1832,22 +1753,11 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
if (ret)
return ret;

- /* Stop here if there is no reset */
- 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;
+ /* Register auxiliary reset driver when applicable */
+ if (data->rst_drvname)
+ ret = devm_meson_clk_rst_aux_register(dev, map, data->rst_drvname);

- return devm_reset_controller_register(dev, &rst->rstc);
+ return ret;
}

static const struct audioclk_data axg_audioclk_data = {
@@ -1866,8 +1776,7 @@ static const struct audioclk_data g12a_audioclk_data = {
.hws = g12a_audio_hw_clks,
.num = ARRAY_SIZE(g12a_audio_hw_clks),
},
- .reset_offset = AUDIO_SW_RESET,
- .reset_num = 26,
+ .rst_drvname = "rst-g12a",
};

static const struct audioclk_data sm1_audioclk_data = {
@@ -1877,8 +1786,7 @@ static const struct audioclk_data sm1_audioclk_data = {
.hws = sm1_audio_hw_clks,
.num = ARRAY_SIZE(sm1_audio_hw_clks),
},
- .reset_offset = AUDIO_SM1_SW_RESET0,
- .reset_num = 39,
+ .rst_drvname = "rst-sm1",
};

static const struct of_device_id clkc_match_table[] = {
--
2.43.0


2024-05-16 15:10:53

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 3/9] reset: amlogic: split the device and platform probe

To prepare the addition of the auxiliary device support, split
the device probe from the probe of the platform device.

The device probe will be common to both the platform and auxiliary
driver.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 55 +++++++++++++++++++++++--------------
1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index 59126c9f194a..fec55321b52b 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -87,6 +87,27 @@ static const struct reset_control_ops meson_reset_ops = {
.deassert = meson_reset_deassert,
};

+static int meson_reset_probe(struct device *dev, struct regmap *map,
+ const struct meson_reset_param *param)
+{
+ unsigned int stride = regmap_get_reg_stride(map);
+ struct meson_reset *data;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->param = param;
+ data->map = map;
+ data->rcdev.owner = dev->driver->owner;
+ data->rcdev.nr_resets = param->reg_count * BITS_PER_BYTE
+ * stride;
+ data->rcdev.ops = &meson_reset_ops;
+ data->rcdev.of_node = dev->of_node;
+
+ return devm_reset_controller_register(dev, &data->rcdev);
+}
+
static const struct meson_reset_param meson8b_param = {
.reg_count = 8,
.reset_offset = 0x0,
@@ -125,46 +146,38 @@ static const struct regmap_config regmap_config = {
.reg_stride = 4,
};

-static int meson_reset_probe(struct platform_device *pdev)
+static int meson_reset_pltf_probe(struct platform_device *pdev)
{
+
+ const struct meson_reset_param *param;
struct device *dev = &pdev->dev;
- struct meson_reset *data;
+ struct regmap *map;
void __iomem *base;

- data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);

- data->param = of_device_get_match_data(dev);
- if (!data->param)
+ param = of_device_get_match_data(dev);
+ if (!param)
return -ENODEV;

- data->map = devm_regmap_init_mmio(dev, base, &regmap_config);
- if (IS_ERR(data->map))
- return dev_err_probe(dev, PTR_ERR(data->map),
+ map = devm_regmap_init_mmio(dev, base, &regmap_config);
+ if (IS_ERR(map))
+ return dev_err_probe(dev, PTR_ERR(map),
"can't init regmap mmio region\n");

- data->rcdev.owner = THIS_MODULE;
- data->rcdev.nr_resets = data->param->reg_count * BITS_PER_BYTE
- * regmap_config.reg_stride;
- data->rcdev.ops = &meson_reset_ops;
- data->rcdev.of_node = dev->of_node;
-
- return devm_reset_controller_register(dev, &data->rcdev);
+ return meson_reset_probe(dev, map, param);
}

-static struct platform_driver meson_reset_driver = {
- .probe = meson_reset_probe,
+static struct platform_driver meson_reset_pltf_driver = {
+ .probe = meson_reset_pltf_probe,
.driver = {
.name = "meson_reset",
.of_match_table = meson_reset_dt_ids,
},
};
-module_platform_driver(meson_reset_driver);
+module_platform_driver(meson_reset_pltf_driver);

MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
MODULE_AUTHOR("Neil Armstrong <[email protected]>");
--
2.43.0


2024-05-16 15:11:44

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 6/9] reset: amlogic: add toggle reset support

Add the emulation for the reset callback using level reset if reset is not
directly supported. This is done to keep the functionality of reset
driver of audio clock controller. This is expected to work by the related
reset consumers.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/reset-meson.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index 65ba9190cb53..e34a10b15593 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -17,6 +17,7 @@
#include <linux/types.h>

struct meson_reset_param {
+ const struct reset_control_ops *reset_ops;
unsigned int reset_num;
int reset_offset;
int level_offset;
@@ -98,6 +99,18 @@ static int meson_reset_deassert(struct reset_controller_dev *rcdev,
return meson_reset_level(rcdev, id, false);
}

+static int meson_reset_level_toggle(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ int ret;
+
+ ret = meson_reset_assert(rcdev, id);
+ if (ret)
+ return ret;
+
+ return meson_reset_deassert(rcdev, id);
+}
+
static const struct reset_control_ops meson_reset_ops = {
.reset = meson_reset_reset,
.assert = meson_reset_assert,
@@ -105,6 +118,13 @@ static const struct reset_control_ops meson_reset_ops = {
.status = meson_reset_status,
};

+static const struct reset_control_ops meson_reset_toggle_ops = {
+ .reset = meson_reset_level_toggle,
+ .assert = meson_reset_assert,
+ .deassert = meson_reset_deassert,
+ .status = meson_reset_status,
+};
+
static int meson_reset_probe(struct device *dev, struct regmap *map,
const struct meson_reset_param *param)
{
@@ -118,13 +138,14 @@ static int meson_reset_probe(struct device *dev, struct regmap *map,
data->map = map;
data->rcdev.owner = dev->driver->owner;
data->rcdev.nr_resets = param->reset_num;
- data->rcdev.ops = &meson_reset_ops;
+ data->rcdev.ops = param->reset_ops;
data->rcdev.of_node = dev->of_node;

return devm_reset_controller_register(dev, &data->rcdev);
}

static const struct meson_reset_param meson8b_param = {
+ .reset_ops = &meson_reset_ops,
.reset_num = 256,
.reset_offset = 0x0,
.level_offset = 0x7c,
@@ -132,6 +153,7 @@ static const struct meson_reset_param meson8b_param = {
};

static const struct meson_reset_param meson_a1_param = {
+ .reset_ops = &meson_reset_ops,
.reset_num = 96,
.reset_offset = 0x0,
.level_offset = 0x40,
@@ -139,6 +161,7 @@ static const struct meson_reset_param meson_a1_param = {
};

static const struct meson_reset_param meson_s4_param = {
+ .reset_ops = &meson_reset_ops,
.reset_num = 192,
.reset_offset = 0x0,
.level_offset = 0x40,
--
2.43.0


2024-05-16 15:21:20

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 7/9] reset: amlogic: add auxiliary reset driver support

Add support for the reset controller present in the audio clock
controller of the g12 and sm1 SoC families, using the auxiliary bus.

This is expected to replace the driver currently present directly
within the related clock driver.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/reset/Kconfig | 1 +
drivers/reset/reset-meson.c | 46 ++++++++++++++++++-
include/soc/amlogic/meson8b-auxiliary-reset.h | 17 +++++++
3 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 include/soc/amlogic/meson8b-auxiliary-reset.h

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 85b27c42cf65..4ceb4dc48fbc 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -134,6 +134,7 @@ config RESET_MCHP_SPARX5
config RESET_MESON
tristate "Meson Reset Driver"
depends on ARCH_MESON || COMPILE_TEST
+ depends on AUXILIARY_BUS
default ARCH_MESON
help
This enables the reset driver for Amlogic Meson SoCs.
diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
index e34a10b15593..b5ddb85296ec 100644
--- a/drivers/reset/reset-meson.c
+++ b/drivers/reset/reset-meson.c
@@ -5,6 +5,7 @@
* Copyright (c) 2016 BayLibre, SAS.
* Author: Neil Armstrong <[email protected]>
*/
+#include <linux/auxiliary_bus.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
@@ -16,6 +17,8 @@
#include <linux/slab.h>
#include <linux/types.h>

+#include <soc/amlogic/meson8b-auxiliary-reset.h>
+
struct meson_reset_param {
const struct reset_control_ops *reset_ops;
unsigned int reset_num;
@@ -218,6 +221,47 @@ static struct platform_driver meson_reset_pltf_driver = {
};
module_platform_driver(meson_reset_pltf_driver);

-MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
+static const struct meson_reset_param meson_g12a_audio_param = {
+ .reset_ops = &meson_reset_toggle_ops,
+ .reset_num = 26,
+ .level_offset = 0x24,
+};
+
+static const struct meson_reset_param meson_sm1_audio_param = {
+ .reset_ops = &meson_reset_toggle_ops,
+ .reset_num = 39,
+ .level_offset = 0x28,
+};
+
+static const struct auxiliary_device_id meson_reset_aux_ids[] = {
+ {
+ .name = "axg-audio-clkc.rst-g12a",
+ .driver_data = (kernel_ulong_t)&meson_g12a_audio_param,
+ }, {
+ .name = "axg-audio-clkc.rst-sm1",
+ .driver_data = (kernel_ulong_t)&meson_sm1_audio_param,
+ },
+};
+MODULE_DEVICE_TABLE(auxiliary, meson_reset_aux_ids);
+
+static int meson_reset_aux_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ const struct meson_reset_param *param =
+ (const struct meson_reset_param *)(id->driver_data);
+ struct meson8b_reset_adev *raux =
+ to_meson8b_reset_adev(adev);
+
+ return meson_reset_probe(&adev->dev, raux->map, param);
+}
+
+static struct auxiliary_driver meson_reset_aux_driver = {
+ .probe = meson_reset_aux_probe,
+ .id_table = meson_reset_aux_ids,
+};
+module_auxiliary_driver(meson_reset_aux_driver);
+
+MODULE_DESCRIPTION("Amlogic Meson Reset driver");
MODULE_AUTHOR("Neil Armstrong <[email protected]>");
+MODULE_AUTHOR("Jerome Brunet <[email protected]>");
MODULE_LICENSE("Dual BSD/GPL");
diff --git a/include/soc/amlogic/meson8b-auxiliary-reset.h b/include/soc/amlogic/meson8b-auxiliary-reset.h
new file mode 100644
index 000000000000..0a465deb4440
--- /dev/null
+++ b/include/soc/amlogic/meson8b-auxiliary-reset.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __SOC_AMLOGIC_MESON8B_AUX_RESET_H
+#define __SOC_AMLOGIC_MESON8B_AUX_RESET_H
+
+#include <linux/auxiliary_bus.h>
+#include <linux/container_of.h>
+#include <linux/regmap.h>
+
+struct meson8b_reset_adev {
+ struct auxiliary_device adev;
+ struct regmap *map;
+};
+
+#define to_meson8b_reset_adev(_adev) \
+ container_of((_adev), struct meson8b_reset_adev, adev)
+
+#endif /* __SOC_AMLOGIC_MESON8B_AUX_RESET_H */
--
2.43.0


2024-05-16 15:39:38

by Jerome Brunet

[permalink] [raw]
Subject: [RFC PATCH 8/9] clk: meson: add auxiliary reset helper driver

Add an helper module to register auxiliary reset drivers from
Amlogic clock controller.

Signed-off-by: Jerome Brunet <[email protected]>
---
drivers/clk/meson/Kconfig | 5 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/meson-clk-rst-aux.c | 84 +++++++++++++++++++++++++++
drivers/clk/meson/meson-clk-rst-aux.h | 14 +++++
4 files changed, 104 insertions(+)
create mode 100644 drivers/clk/meson/meson-clk-rst-aux.c
create mode 100644 drivers/clk/meson/meson-clk-rst-aux.h

diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 59a40a49f8e1..6905aa2f080c 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -37,6 +37,11 @@ config COMMON_CLK_MESON_VCLK
config COMMON_CLK_MESON_CLKC_UTILS
tristate

+config COMMON_CLK_MESON_CLK_RST_AUX
+ depends on REGMAP && AUXILIARY_BUS
+ imply RESET_MESON
+ 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 9ba43fe7a07a..2926a04b6c33 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -2,6 +2,7 @@
# Amlogic clock drivers

obj-$(CONFIG_COMMON_CLK_MESON_CLKC_UTILS) += meson-clkc-utils.o
+obj-$(CONFIG_COMMON_CLK_MESON_CLK_RST_AUX) += meson-clk-rst-aux.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-clk-rst-aux.c b/drivers/clk/meson/meson-clk-rst-aux.c
new file mode 100644
index 000000000000..a7cf3c39828c
--- /dev/null
+++ b/drivers/clk/meson/meson-clk-rst-aux.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) 2024 BayLibre, SAS.
+// Author: Jerome Brunet <[email protected]>
+
+#include <linux/auxiliary_bus.h>
+#include <linux/idr.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include <soc/amlogic/meson8b-auxiliary-reset.h>
+
+#include "meson-clk-rst-aux.h"
+
+static DEFINE_IDA(meson_clk_rst_aux_ida);
+
+static void meson_clk_rst_aux_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+ struct meson8b_reset_adev *raux =
+ to_meson8b_reset_adev(adev);
+
+ ida_free(&meson_clk_rst_aux_ida, adev->id);
+ kfree(raux);
+}
+
+static void meson_clk_rst_aux_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+int devm_meson_clk_rst_aux_register(struct device *dev,
+ struct regmap *map,
+ const char *adev_name)
+{
+ struct meson8b_reset_adev *raux;
+ struct auxiliary_device *adev;
+ int ret;
+
+ raux = kzalloc(sizeof(*raux), GFP_KERNEL);
+ if (!raux)
+ return -ENOMEM;
+
+ ret = ida_alloc(&meson_clk_rst_aux_ida, GFP_KERNEL);
+ if (ret < 0)
+ goto raux_free;
+
+ raux->map = map;
+
+ adev = &raux->adev;
+ adev->id = ret;
+ adev->name = adev_name;
+ adev->dev.parent = dev;
+ adev->dev.release = meson_clk_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_clk_rst_aux_unregister_adev,
+ adev);
+
+ida_free:
+ ida_free(&meson_clk_rst_aux_ida, adev->id);
+raux_free:
+ kfree(raux);
+ return ret;
+
+}
+EXPORT_SYMBOL_GPL(devm_meson_clk_rst_aux_register);
+
+MODULE_DESCRIPTION("Amlogic auxiliary reset helper for clock");
+MODULE_AUTHOR("Jerome Brunet <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/clk/meson/meson-clk-rst-aux.h b/drivers/clk/meson/meson-clk-rst-aux.h
new file mode 100644
index 000000000000..386a55a36cd9
--- /dev/null
+++ b/drivers/clk/meson/meson-clk-rst-aux.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 BayLibre, SAS.
+ * Author: Jerome Brunet <[email protected]>
+ */
+
+#ifndef __MESON_CLK_RST_AUX_H
+#define __MESON_CLK_RST_AUX_H
+
+int devm_meson_clk_rst_aux_register(struct device *dev,
+ struct regmap *map,
+ const char *adev_name);
+
+#endif /* __MESON_CLK_RST_AUX_H */
--
2.43.0


2024-05-18 01:29:28

by Jan Dakinevich

[permalink] [raw]
Subject: Re: [RFC PATCH 7/9] reset: amlogic: add auxiliary reset driver support



On 5/16/24 18:08, Jerome Brunet wrote:
> Add support for the reset controller present in the audio clock
> controller of the g12 and sm1 SoC families, using the auxiliary bus.
>
> This is expected to replace the driver currently present directly
> within the related clock driver.
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> drivers/reset/Kconfig | 1 +
> drivers/reset/reset-meson.c | 46 ++++++++++++++++++-
> include/soc/amlogic/meson8b-auxiliary-reset.h | 17 +++++++
> 3 files changed, 63 insertions(+), 1 deletion(-)
> create mode 100644 include/soc/amlogic/meson8b-auxiliary-reset.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 85b27c42cf65..4ceb4dc48fbc 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -134,6 +134,7 @@ config RESET_MCHP_SPARX5
> config RESET_MESON
> tristate "Meson Reset Driver"
> depends on ARCH_MESON || COMPILE_TEST
> + depends on AUXILIARY_BUS

I don't understand, who enables AUXILIARY_BUS. If I'm not mistaken,
AUXILIARY_BUS should be selected by something that is going to use it,
and it is not intended for defconfig.

> default ARCH_MESON
> help
> This enables the reset driver for Amlogic Meson SoCs.
> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> index e34a10b15593..b5ddb85296ec 100644
> --- a/drivers/reset/reset-meson.c
> +++ b/drivers/reset/reset-meson.c
> @@ -5,6 +5,7 @@
> * Copyright (c) 2016 BayLibre, SAS.
> * Author: Neil Armstrong <[email protected]>
> */
> +#include <linux/auxiliary_bus.h>
> #include <linux/err.h>
> #include <linux/init.h>
> #include <linux/io.h>
> @@ -16,6 +17,8 @@
> #include <linux/slab.h>
> #include <linux/types.h>
>
> +#include <soc/amlogic/meson8b-auxiliary-reset.h>
> +
> struct meson_reset_param {
> const struct reset_control_ops *reset_ops;
> unsigned int reset_num;
> @@ -218,6 +221,47 @@ static struct platform_driver meson_reset_pltf_driver = {
> };
> module_platform_driver(meson_reset_pltf_driver);
>
> -MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
> +static const struct meson_reset_param meson_g12a_audio_param = {
> + .reset_ops = &meson_reset_toggle_ops,
> + .reset_num = 26,
> + .level_offset = 0x24,
> +};
> +
> +static const struct meson_reset_param meson_sm1_audio_param = {
> + .reset_ops = &meson_reset_toggle_ops,
> + .reset_num = 39,
> + .level_offset = 0x28,
> +};
> +
> +static const struct auxiliary_device_id meson_reset_aux_ids[] = {
> + {
> + .name = "axg-audio-clkc.rst-g12a",
> + .driver_data = (kernel_ulong_t)&meson_g12a_audio_param,
> + }, {
> + .name = "axg-audio-clkc.rst-sm1",
> + .driver_data = (kernel_ulong_t)&meson_sm1_audio_param,
> + },
> +};
> +MODULE_DEVICE_TABLE(auxiliary, meson_reset_aux_ids);
> +
> +static int meson_reset_aux_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + const struct meson_reset_param *param =
> + (const struct meson_reset_param *)(id->driver_data);
> + struct meson8b_reset_adev *raux =
> + to_meson8b_reset_adev(adev);
> +
> + return meson_reset_probe(&adev->dev, raux->map, param);
> +}
> +
> +static struct auxiliary_driver meson_reset_aux_driver = {
> + .probe = meson_reset_aux_probe,
> + .id_table = meson_reset_aux_ids,
> +};
> +module_auxiliary_driver(meson_reset_aux_driver);
> +
> +MODULE_DESCRIPTION("Amlogic Meson Reset driver");
> MODULE_AUTHOR("Neil Armstrong <[email protected]>");
> +MODULE_AUTHOR("Jerome Brunet <[email protected]>");
> MODULE_LICENSE("Dual BSD/GPL");
> diff --git a/include/soc/amlogic/meson8b-auxiliary-reset.h b/include/soc/amlogic/meson8b-auxiliary-reset.h
> new file mode 100644
> index 000000000000..0a465deb4440
> --- /dev/null
> +++ b/include/soc/amlogic/meson8b-auxiliary-reset.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __SOC_AMLOGIC_MESON8B_AUX_RESET_H
> +#define __SOC_AMLOGIC_MESON8B_AUX_RESET_H
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/container_of.h>
> +#include <linux/regmap.h>
> +
> +struct meson8b_reset_adev {
> + struct auxiliary_device adev;
> + struct regmap *map;
> +};
> +
> +#define to_meson8b_reset_adev(_adev) \
> + container_of((_adev), struct meson8b_reset_adev, adev)
> +
> +#endif /* __SOC_AMLOGIC_MESON8B_AUX_RESET_H */

--
Best regards
Jan Dakinevich

2024-05-18 01:34:37

by Jan Dakinevich

[permalink] [raw]
Subject: Re: [RFC PATCH 9/9] clk: amlogic: axg-audio: use the auxiliary reset driver

May be I'm doing something wrong, but "git am" fails on this patch on
tag next-20240517.

On 5/16/24 18:08, Jerome Brunet wrote:
> Remove the implementation of the reset driver in axg audio
> clock driver and migrate to the one provided by reset framework
> on auxiliary bus
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> drivers/clk/meson/Kconfig | 1 +
> drivers/clk/meson/axg-audio.c | 108 +++-------------------------------
> 2 files changed, 9 insertions(+), 100 deletions(-)
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 6905aa2f080c..b89e769a6362 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -110,6 +110,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_CLK_RST_AUX
> select REGMAP_MMIO
> help
> Support for the audio clock controller on AmLogic A113D devices,
> diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
> index 93e6a6d321be..dc9191a9ab0f 100644
> --- a/drivers/clk/meson/axg-audio.c
> +++ b/drivers/clk/meson/axg-audio.c
> @@ -15,6 +15,7 @@
> #include <linux/reset-controller.h>
> #include <linux/slab.h>
>
> +#include "meson-clk-rst-aux.h"
> #include "meson-clkc-utils.h"
> #include "axg-audio.h"
> #include "clk-regmap.h"
> @@ -1678,84 +1679,6 @@ static struct clk_regmap *const sm1_clk_regmaps[] = {
> &sm1_earcrx_dmac_clk,
> };
>
> -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,
> @@ -1766,15 +1689,13 @@ struct audioclk_data {
> struct clk_regmap *const *regmap_clks;
> unsigned int regmap_clk_num;
> struct meson_clk_hw_data hw_clks;
> - unsigned int reset_offset;
> - unsigned int reset_num;
> + const char *rst_drvname;
> };
>
> 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;
> @@ -1832,22 +1753,11 @@ static int axg_audio_clkc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - /* Stop here if there is no reset */
> - 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;
> + /* Register auxiliary reset driver when applicable */
> + if (data->rst_drvname)
> + ret = devm_meson_clk_rst_aux_register(dev, map, data->rst_drvname);
>
> - return devm_reset_controller_register(dev, &rst->rstc);
> + return ret;
> }
>
> static const struct audioclk_data axg_audioclk_data = {
> @@ -1866,8 +1776,7 @@ static const struct audioclk_data g12a_audioclk_data = {
> .hws = g12a_audio_hw_clks,
> .num = ARRAY_SIZE(g12a_audio_hw_clks),
> },
> - .reset_offset = AUDIO_SW_RESET,
> - .reset_num = 26,
> + .rst_drvname = "rst-g12a",
> };
>
> static const struct audioclk_data sm1_audioclk_data = {
> @@ -1877,8 +1786,7 @@ static const struct audioclk_data sm1_audioclk_data = {
> .hws = sm1_audio_hw_clks,
> .num = ARRAY_SIZE(sm1_audio_hw_clks),
> },
> - .reset_offset = AUDIO_SM1_SW_RESET0,
> - .reset_num = 39,
> + .rst_drvname = "rst-sm1",
> };
>
> static const struct of_device_id clkc_match_table[] = {

--
Best regards
Jan Dakinevich

2024-05-18 01:44:07

by Jan Dakinevich

[permalink] [raw]
Subject: Re: [RFC PATCH 0/9] reset: amlogic: move reset drivers out of CCF

This is a nice rework! But unfortunately after applying these patches at
least USB doesn't work on A1 SoC. My assumption it is because
RESET_MESON is now unselected due to dependency on AUXILIARY_BUS.

On 5/16/24 18:08, Jerome Brunet wrote:
> This RFC follows the discussion about having reset driver in the clock tree
> [1]. Ideally those should reside in the reset part of tree.
>
> Also the code of the amlogic reset driver is very similar between the 2 trees
> and could use the same driver code.
>
> This RFC moves the reset driver of audio clock controller of the g12 and
> sm1 SoC family to the reset tree, using the auxiliary bus.
>
> The infrastructure put in place is meant to be generic enough so we may
> eventually also move the reset drivers in the meson8b and aoclk clock
> controllers.
>
> [1] https://lore.kernel.org/linux-clk/[email protected]
>
> Jerome Brunet (9):
> reset: amlogic: convert driver to regmap
> reset: amlogic: add driver parameters
> reset: amlogic: split the device and platform probe
> reset: amlogic: use reset number instead of register count
> reset: amlogic: add reset status support
> reset: amlogic: add toggle reset support
> reset: amlogic: add auxiliary reset driver support
> clk: meson: add auxiliary reset helper driver
> clk: amlogic: axg-audio: use the auxiliary reset driver
>
> drivers/clk/meson/Kconfig | 6 +
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/axg-audio.c | 108 +--------
> drivers/clk/meson/meson-clk-rst-aux.c | 84 +++++++
> drivers/clk/meson/meson-clk-rst-aux.h | 14 ++
> drivers/reset/Kconfig | 1 +
> drivers/reset/reset-meson.c | 210 ++++++++++++++----
> include/soc/amlogic/meson8b-auxiliary-reset.h | 17 ++
> 8 files changed, 293 insertions(+), 148 deletions(-)
> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.c
> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.h
> create mode 100644 include/soc/amlogic/meson8b-auxiliary-reset.h
>

--
Best regards
Jan Dakinevich

2024-05-30 01:01:57

by Stephen Boyd

[permalink] [raw]
Subject: Re: [RFC PATCH 8/9] clk: meson: add auxiliary reset helper driver

Quoting Jerome Brunet (2024-05-16 08:08:38)
> Add an helper module to register auxiliary reset drivers from
> Amlogic clock controller.
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> drivers/clk/meson/Kconfig | 5 ++
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/meson-clk-rst-aux.c | 84 +++++++++++++++++++++++++++
> drivers/clk/meson/meson-clk-rst-aux.h | 14 +++++
> 4 files changed, 104 insertions(+)
> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.c
> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.h
>
> diff --git a/drivers/clk/meson/meson-clk-rst-aux.h b/drivers/clk/meson/meson-clk-rst-aux.h
> new file mode 100644
> index 000000000000..386a55a36cd9
> --- /dev/null
> +++ b/drivers/clk/meson/meson-clk-rst-aux.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2024 BayLibre, SAS.
> + * Author: Jerome Brunet <[email protected]>
> + */
> +
> +#ifndef __MESON_CLK_RST_AUX_H
> +#define __MESON_CLK_RST_AUX_H
> +
> +int devm_meson_clk_rst_aux_register(struct device *dev,
> + struct regmap *map,
> + const char *adev_name);

I'd prefer we move the device creation and registration logic to
drivers/reset as well. See commit 098c290a490d ("clock, reset:
microchip: move all mpfs reset code to the reset subsystem") for some
inspiration.

One thing I haven't really thought about too much is if they're two
different modules. One for clk and one for reset. If the device
registration API is a symbol the clk module depends on then maybe that
is better because it means both modules are loaded, avoiding a
round-trip through modprobe. It also makes sure that the drivers are
either both builtin or both modular.

2024-06-10 10:11:17

by Jerome Brunet

[permalink] [raw]
Subject: Re: [RFC PATCH 8/9] clk: meson: add auxiliary reset helper driver

On Wed 29 May 2024 at 18:01, Stephen Boyd <[email protected]> wrote:

> Quoting Jerome Brunet (2024-05-16 08:08:38)
>> Add an helper module to register auxiliary reset drivers from
>> Amlogic clock controller.
>>
>> Signed-off-by: Jerome Brunet <[email protected]>
>> ---
>> drivers/clk/meson/Kconfig | 5 ++
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/meson-clk-rst-aux.c | 84 +++++++++++++++++++++++++++
>> drivers/clk/meson/meson-clk-rst-aux.h | 14 +++++
>> 4 files changed, 104 insertions(+)
>> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.c
>> create mode 100644 drivers/clk/meson/meson-clk-rst-aux.h
>>
>> diff --git a/drivers/clk/meson/meson-clk-rst-aux.h b/drivers/clk/meson/meson-clk-rst-aux.h
>> new file mode 100644
>> index 000000000000..386a55a36cd9
>> --- /dev/null
>> +++ b/drivers/clk/meson/meson-clk-rst-aux.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2024 BayLibre, SAS.
>> + * Author: Jerome Brunet <[email protected]>
>> + */
>> +
>> +#ifndef __MESON_CLK_RST_AUX_H
>> +#define __MESON_CLK_RST_AUX_H
>> +
>> +int devm_meson_clk_rst_aux_register(struct device *dev,
>> + struct regmap *map,
>> + const char *adev_name);
>
> I'd prefer we move the device creation and registration logic to
> drivers/reset as well. See commit 098c290a490d ("clock, reset:
> microchip: move all mpfs reset code to the reset subsystem") for some
> inspiration.

Ok but if it lives in reset I don't really get the purpose served by the
auxiliary devices in that case. Why not export a function that directly
calls reset_controller_register() in that case ?

I thought the point was to properly decouple both sides.

I don't have strong opinion about it, TBH. It is just how it made sense
to me. If you are sure about this, I don't mind changing

>
> One thing I haven't really thought about too much is if they're two
> different modules. One for clk and one for reset. If the device
> registration API is a symbol the clk module depends on then maybe that
> is better because it means both modules are loaded, avoiding a
> round-trip through modprobe. It also makes sure that the drivers are
> either both builtin or both modular.

I have checked with the current implementation, if the reset driver is
missing, the clock part does not fail. Registering the aux device
succeeds in clock but the device never comes up (duh). So it does
not crash, the consumers of the aux reset device will just defer.

Said differently, the '#if IS_ENABLED(CONFIG_RESET_CONTROLLER)' in
clk-mpfs.c was not necessary ... it was removed in the changed you
linked anyway.

(Sorry Stephen, you got it twice ... missed the Reply-all the first time
around)

--
Jerome