1. Patchew
  2. linux
  3. [PATCH 0/5] imx8mp: Add support to Run/Stall DSP via reset API
  4. View patch

[PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits

Daniel Baluta posted 5 patches 10 months ago
There is a newer version of this series
[PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits
Posted by Daniel Baluta 10 months ago 
Current code supports EARC PHY Software Reset and EARC 	Software
Reset but it is not easily extensible to more reset bits.

So, refactor the code in order to easily allow more reset bits
in the future.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 drivers/reset/reset-imx8mp-audiomix.c | 53 ++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index 1fe21980a66c..6b1666c4e069 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -12,7 +12,30 @@
 #include <linux/reset-controller.h>
 
 #define IMX8MP_AUDIOMIX_EARC_OFFSET		0x200
-#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x3
+#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x1
+#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	0x2
+
+#define IMX8MP_AUDIOMIX_EARC		0
+#define IMX8MP_AUDIOMIX_EARC_PHY	1
+
+#define IMX8MP_AUDIOMIX_RESET_NUM	2
+
+struct imx8mp_reset_map {
+	unsigned int offset;
+	unsigned int mask;
+};
+
+static const struct imx8mp_reset_map reset_map[IMX8MP_AUDIOMIX_RESET_NUM] = {
+	[IMX8MP_AUDIOMIX_EARC] = {
+		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
+		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
+	},
+	[IMX8MP_AUDIOMIX_EARC_PHY] = {
+		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
+		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
+	},
+
+};
 
 struct imx8mp_audiomix_reset {
 	struct reset_controller_dev rcdev;
@@ -30,13 +53,18 @@ static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
 {
 	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
 	void __iomem *reg_addr = priv->base;
-	unsigned int mask, reg;
+	unsigned int mask, offset, reg;
 	unsigned long flags;
 
-	mask = BIT(id);
+	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)
+		return -EINVAL;
+
+	mask = reset_map[id].mask;
+	offset = reset_map[id].offset;
+
 	spin_lock_irqsave(&priv->lock, flags);
-	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
-	writel(reg & ~mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
+	reg = readl(reg_addr + offset);
+	writel(reg & ~mask, reg_addr + offset);
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return 0;
@@ -47,13 +75,18 @@ static int imx8mp_audiomix_reset_deassert(struct reset_controller_dev *rcdev,
 {
 	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
 	void __iomem *reg_addr = priv->base;
-	unsigned int mask, reg;
+	unsigned int mask, offset, reg;
 	unsigned long flags;
 
-	mask = BIT(id);
+	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)
+		return -EINVAL;
+
+	mask = reset_map[id].mask;
+	offset = reset_map[id].offset;
+
 	spin_lock_irqsave(&priv->lock, flags);
-	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
-	writel(reg | mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
+	reg = readl(reg_addr + offset);
+	writel(reg | mask, reg_addr + offset);
 	spin_unlock_irqrestore(&priv->lock, flags);
 
 	return 0;
@@ -78,7 +111,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
 	spin_lock_init(&priv->lock);
 
 	priv->rcdev.owner     = THIS_MODULE;
-	priv->rcdev.nr_resets = fls(IMX8MP_AUDIOMIX_EARC_RESET_MASK);
+	priv->rcdev.nr_resets = IMX8MP_AUDIOMIX_RESET_NUM;
 	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
 	priv->rcdev.of_node   = dev->parent->of_node;
 	priv->rcdev.dev	      = dev;
-- 
2.25.1
Re: [PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits
Posted by Frank Li 10 months ago 
On Tue, Feb 18, 2025 at 10:57:09AM +0200, Daniel Baluta wrote:
> Current code supports EARC PHY Software Reset and EARC 	Software
> Reset but it is not easily extensible to more reset bits.
>
> So, refactor the code in order to easily allow more reset bits
> in the future.

Nit: wrap at 75 chars

>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 53 ++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index 1fe21980a66c..6b1666c4e069 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -12,7 +12,30 @@
>  #include <linux/reset-controller.h>
>
>  #define IMX8MP_AUDIOMIX_EARC_OFFSET		0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x3
> +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x1
> +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	0x2
> +
> +#define IMX8MP_AUDIOMIX_EARC		0
> +#define IMX8MP_AUDIOMIX_EARC_PHY	1


Does dt binding need such macro?

If not, you can use enum.
If yes, create binding header file for it.

Frank
> +
> +#define IMX8MP_AUDIOMIX_RESET_NUM	2
> +
> +struct imx8mp_reset_map {
> +	unsigned int offset;
> +	unsigned int mask;
> +};
> +
> +static const struct imx8mp_reset_map reset_map[IMX8MP_AUDIOMIX_RESET_NUM] = {
> +	[IMX8MP_AUDIOMIX_EARC] = {
> +		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
> +		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
> +	},
> +	[IMX8MP_AUDIOMIX_EARC_PHY] = {
> +		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
> +		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
> +	},
> +
> +};
>
>  struct imx8mp_audiomix_reset {
>  	struct reset_controller_dev rcdev;
> @@ -30,13 +53,18 @@ static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>  	void __iomem *reg_addr = priv->base;
> -	unsigned int mask, reg;
> +	unsigned int mask, offset, reg;
>  	unsigned long flags;
>
> -	mask = BIT(id);
> +	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)
> +		return -EINVAL;
> +
> +	mask = reset_map[id].mask;
> +	offset = reset_map[id].offset;
> +
>  	spin_lock_irqsave(&priv->lock, flags);
> -	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> -	writel(reg & ~mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> +	reg = readl(reg_addr + offset);
> +	writel(reg & ~mask, reg_addr + offset);
>  	spin_unlock_irqrestore(&priv->lock, flags);
>
>  	return 0;
> @@ -47,13 +75,18 @@ static int imx8mp_audiomix_reset_deassert(struct reset_controller_dev *rcdev,
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>  	void __iomem *reg_addr = priv->base;
> -	unsigned int mask, reg;
> +	unsigned int mask, offset, reg;
>  	unsigned long flags;
>
> -	mask = BIT(id);
> +	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)
> +		return -EINVAL;
> +
> +	mask = reset_map[id].mask;
> +	offset = reset_map[id].offset;
> +
>  	spin_lock_irqsave(&priv->lock, flags);
> -	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> -	writel(reg | mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> +	reg = readl(reg_addr + offset);
> +	writel(reg | mask, reg_addr + offset);
>  	spin_unlock_irqrestore(&priv->lock, flags);
>
>  	return 0;
> @@ -78,7 +111,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  	spin_lock_init(&priv->lock);
>
>  	priv->rcdev.owner     = THIS_MODULE;
> -	priv->rcdev.nr_resets = fls(IMX8MP_AUDIOMIX_EARC_RESET_MASK);
> +	priv->rcdev.nr_resets = IMX8MP_AUDIOMIX_RESET_NUM;
>  	priv->rcdev.ops       = &imx8mp_audiomix_reset_ops;
>  	priv->rcdev.of_node   = dev->parent->of_node;
>  	priv->rcdev.dev	      = dev;
> --
> 2.25.1
>
Re: [PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits
Posted by Daniel Baluta 10 months ago 
On Tue, Feb 18, 2025 at 5:56 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Tue, Feb 18, 2025 at 10:57:09AM +0200, Daniel Baluta wrote:
> > Current code supports EARC PHY Software Reset and EARC        Software
> > Reset but it is not easily extensible to more reset bits.
> >
> > So, refactor the code in order to easily allow more reset bits
> > in the future.
>
> Nit: wrap at 75 chars
>
> >
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> > ---
> >  drivers/reset/reset-imx8mp-audiomix.c | 53 ++++++++++++++++++++++-----
> >  1 file changed, 43 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> > index 1fe21980a66c..6b1666c4e069 100644
> > --- a/drivers/reset/reset-imx8mp-audiomix.c
> > +++ b/drivers/reset/reset-imx8mp-audiomix.c
> > @@ -12,7 +12,30 @@
> >  #include <linux/reset-controller.h>
> >
> >  #define IMX8MP_AUDIOMIX_EARC_OFFSET          0x200
> > -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK              0x3
> > +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK              0x1
> > +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK  0x2
> > +
> > +#define IMX8MP_AUDIOMIX_EARC         0
> > +#define IMX8MP_AUDIOMIX_EARC_PHY     1
>
>
> Does dt binding need such macro?
>
> If not, you can use enum.
> If yes, create binding header file for it.

Thanks Philipp, Peng and Frank for your comments. All are valid.

Will fix them in v2.
Re: [PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits
Posted by Peng Fan 10 months ago 
On Tue, Feb 18, 2025 at 10:57:09AM +0200, Daniel Baluta wrote:
>Current code supports EARC PHY Software Reset and EARC 	Software
>Reset but it is not easily extensible to more reset bits.
>
>So, refactor the code in order to easily allow more reset bits
>in the future.
>
>Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>---
> drivers/reset/reset-imx8mp-audiomix.c | 53 ++++++++++++++++++++++-----
> 1 file changed, 43 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>index 1fe21980a66c..6b1666c4e069 100644
>--- a/drivers/reset/reset-imx8mp-audiomix.c
>+++ b/drivers/reset/reset-imx8mp-audiomix.c
>@@ -12,7 +12,30 @@
> #include <linux/reset-controller.h>
> 
> #define IMX8MP_AUDIOMIX_EARC_OFFSET		0x200
>-#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x3
>+#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x1
>+#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	0x2
>+
>+#define IMX8MP_AUDIOMIX_EARC		0
>+#define IMX8MP_AUDIOMIX_EARC_PHY	1

Should this two lines be put in dt-bindings?

>+
>+#define IMX8MP_AUDIOMIX_RESET_NUM	2
>+
>+struct imx8mp_reset_map {
>+	unsigned int offset;
>+	unsigned int mask;
>+};
>+
>+static const struct imx8mp_reset_map reset_map[IMX8MP_AUDIOMIX_RESET_NUM] = {
>+	[IMX8MP_AUDIOMIX_EARC] = {
>+		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
>+		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
>+	},
>+	[IMX8MP_AUDIOMIX_EARC_PHY] = {
>+		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
>+		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
>+	},
>+

blank line

>+};
> 

Regards,
Peng
Re: [PATCH 2/5] reset: imx8mp-audiomix: Prepare the code for more reset bits
Posted by Philipp Zabel 10 months ago 
Hi Daniel,

On Di, 2025-02-18 at 10:57 +0200, Daniel Baluta wrote:
> Current code supports EARC PHY Software Reset and EARC 	Software
> Reset but it is not easily extensible to more reset bits.
> 
> So, refactor the code in order to easily allow more reset bits
> in the future.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 53 ++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index 1fe21980a66c..6b1666c4e069 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -12,7 +12,30 @@
>  #include <linux/reset-controller.h>
>  
>  #define IMX8MP_AUDIOMIX_EARC_OFFSET		0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x3
> +#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		0x1
> +#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	0x2

Will any of the reset controls manipulate multiple bits at once?
I'd use BIT(0) and BIT(1) here.

> +
> +#define IMX8MP_AUDIOMIX_EARC		0
> +#define IMX8MP_AUDIOMIX_EARC_PHY	1
> +
> +#define IMX8MP_AUDIOMIX_RESET_NUM	2
> +
> +struct imx8mp_reset_map {
> +	unsigned int offset;
> +	unsigned int mask;
> +};
> +
> +static const struct imx8mp_reset_map reset_map[IMX8MP_AUDIOMIX_RESET_NUM] = {

If you make this reset_map[], drop IMX8MP_AUDIOMIX_RESET_NUM, and use
.nr_resets = ARRAY_SIZE(reset_map) below, followup patches that add new
bits will be simplified.

> +	[IMX8MP_AUDIOMIX_EARC] = {
> +		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
> +		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
> +	},
> +	[IMX8MP_AUDIOMIX_EARC_PHY] = {
> +		.offset	= IMX8MP_AUDIOMIX_EARC_OFFSET,
> +		.mask	= IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK,
> +	},
> +

Please drop this empty line.

> +};
>  
>  struct imx8mp_audiomix_reset {
>  	struct reset_controller_dev rcdev;
> @@ -30,13 +53,18 @@ static int imx8mp_audiomix_reset_assert(struct reset_controller_dev *rcdev,
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>  	void __iomem *reg_addr = priv->base;
> -	unsigned int mask, reg;
> +	unsigned int mask, offset, reg;
>  	unsigned long flags;
>  
> -	mask = BIT(id);
> +	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)

Whitespace error. But also, this check is not necessary.
The reset core will not return reset controls that fail the same check
in of_reset_simple_xlate(), so we can never get here with the wrong id
if rcdev.nr_resets correctly is set to ARRAY_SIZE(reset_map).

> +		return -EINVAL;
> +
> +	mask = reset_map[id].mask;
> +	offset = reset_map[id].offset;
> +
>  	spin_lock_irqsave(&priv->lock, flags);
> -	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> -	writel(reg & ~mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> +	reg = readl(reg_addr + offset);
> +	writel(reg & ~mask, reg_addr + offset);
>  	spin_unlock_irqrestore(&priv->lock, flags);
>  
>  	return 0;
> @@ -47,13 +75,18 @@ static int imx8mp_audiomix_reset_deassert(struct reset_controller_dev *rcdev,
>  {
>  	struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>  	void __iomem *reg_addr = priv->base;
> -	unsigned int mask, reg;
> +	unsigned int mask, offset, reg;
>  	unsigned long flags;
>  
> -	mask = BIT(id);
> +	if (id >=  IMX8MP_AUDIOMIX_RESET_NUM)
> +		return -EINVAL;

Same as above.

> +
> +	mask = reset_map[id].mask;
> +	offset = reset_map[id].offset;
> +
>  	spin_lock_irqsave(&priv->lock, flags);
> -	reg = readl(reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> -	writel(reg | mask, reg_addr + IMX8MP_AUDIOMIX_EARC_OFFSET);
> +	reg = readl(reg_addr + offset);
> +	writel(reg | mask, reg_addr + offset);
>  	spin_unlock_irqrestore(&priv->lock, flags);
>  
>  	return 0;
> @@ -78,7 +111,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>  	spin_lock_init(&priv->lock);
>  
>  	priv->rcdev.owner     = THIS_MODULE;
> -	priv->rcdev.nr_resets = fls(IMX8MP_AUDIOMIX_EARC_RESET_MASK);
> +	priv->rcdev.nr_resets = IMX8MP_AUDIOMIX_RESET_NUM;

Could use ARRAY_SIZE(reset_map) here.

regards
Philipp

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.