imx.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Laurentiu Mihalcea <laurentiumihalcea111@gmail.com>,
	Abel Vesa	 <abelvesa@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley	 <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Fabio Estevam	 <festevam@gmail.com>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	Shengjiu Wang	 <shengjiu.wang@nxp.com>,
	Frank Li <Frank.Li@nxp.com>
Cc: linux-clk@vger.kernel.org, imx@lists.linux.dev,
	devicetree@vger.kernel.org,
		linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	 Pengutronix Kernel Team	 <kernel@pengutronix.de>
Subject: Re: [PATCH v4 4/8] reset: imx8mp-audiomix: Drop unneeded macros
Date: Wed, 05 Nov 2025 16:10:38 +0100	[thread overview]
Message-ID: <5535d4dbf94b01fe775b5f905afea8c860f9c03a.camel@pengutronix.de> (raw)
In-Reply-To: <20251104120301.913-5-laurentiumihalcea111@gmail.com>

On Di, 2025-11-04 at 04:02 -0800, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> 
> The macros defining the mask values for the EARC, EARC PHY resets,
> and the DSP RUN_STALL signal can be dropped as they are not and will
> not be used anywhere else except to set the value of the "mask" field
> from "struct imx8mp_reset_map". In this particular case, based on the
> name of the "mask" field, you can already deduce what these values are
> for, which is why defining macros for them doesn't offer any new
> information, nor does it help with the code readability.
> 
> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
>  drivers/reset/reset-imx8mp-audiomix.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index eceb37ff5dc5..e9643365a62c 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -14,11 +14,7 @@
>  #include <linux/reset-controller.h>
>  
>  #define IMX8MP_AUDIOMIX_EARC_RESET_OFFSET	0x200
> -#define IMX8MP_AUDIOMIX_EARC_RESET_MASK		BIT(0)
> -#define IMX8MP_AUDIOMIX_EARC_PHY_RESET_MASK	BIT(1)
> -
>  #define IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET	0x108
> -#define IMX8MP_AUDIOMIX_DSP_RUNSTALL_MASK	BIT(5)
>  
>  struct imx8mp_reset_map {
>  	unsigned int offset;
> @@ -29,17 +25,17 @@ struct imx8mp_reset_map {
>  static const struct imx8mp_reset_map reset_map[] = {
>  	[IMX8MP_AUDIOMIX_EARC_RESET] = {
>  		.offset	= IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> -		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
> +		.mask = BIT(0),

Since the masks are all single-bit, and you make that mandatory in the
next patch, I suggest storing the bit offset in the reset map instead:

-		.mask	= IMX8MP_AUDIOMIX_EARC_RESET_MASK,
+		.bit = 0,

and so on. That way it's clear that no reset control ever manipulates
multiple bits at the same time.

Either way, either in this patch or the next, add an

  #include <linux/bits.h>

for BIT().

regards
Philipp

  reply	other threads:[~2025-11-05 15:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-04 12:02 [PATCH v4 0/8] Add support for i.MX8ULP's SIM LPAV Laurentiu Mihalcea
2025-11-04 12:02 ` [PATCH v4 1/8] reset: imx8mp-audiomix: Fix bad mask values Laurentiu Mihalcea
2025-11-05 15:16   ` Philipp Zabel
2025-11-04 12:02 ` [PATCH v4 2/8] dt-bindings: clock: document 8ULP's SIM LPAV Laurentiu Mihalcea
2025-11-04 12:02 ` [PATCH v4 3/8] clk: imx: add driver for imx8ulp's sim lpav Laurentiu Mihalcea
2025-11-04 12:45   ` Peng Fan
2025-11-05  9:36   ` Daniel Baluta
2025-11-04 12:02 ` [PATCH v4 4/8] reset: imx8mp-audiomix: Drop unneeded macros Laurentiu Mihalcea
2025-11-05 15:10   ` Philipp Zabel [this message]
2025-11-04 12:02 ` [PATCH v4 5/8] reset: imx8mp-audiomix: Switch to using regmap API Laurentiu Mihalcea
2025-11-04 16:44   ` Frank Li
2025-11-05 11:53     ` Laurentiu Mihalcea
2025-11-05 15:23       ` Frank Li
2025-11-05 15:10   ` Philipp Zabel
2025-11-04 12:02 ` [PATCH v4 6/8] reset: imx8mp-audiomix: Extend the driver usage Laurentiu Mihalcea
2025-11-04 16:46   ` Frank Li
2025-11-04 12:03 ` [PATCH v4 7/8] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV Laurentiu Mihalcea
2025-11-04 12:03 ` [PATCH v4 8/8] arm64: dts: imx8ulp: add sim lpav node Laurentiu Mihalcea
2025-11-11 16:05 ` [PATCH v4 0/8] Add support for i.MX8ULP's SIM LPAV Abel Vesa
2025-11-11 16:10   ` Abel Vesa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5535d4dbf94b01fe775b5f905afea8c860f9c03a.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=Frank.Li@nxp.com \
    --cc=abelvesa@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel.baluta@nxp.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=laurentiumihalcea111@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=shengjiu.wang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).