devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Laurentiu Mihalcea <laurentiumihalcea111@gmail.com>,
	Frank Li <Frank.li@nxp.com>
Cc: 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>,
		devicetree@vger.kernel.org, imx@lists.linux.dev,
	 linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	 Pengutronix Kernel Team	 <kernel@pengutronix.de>
Subject: Re: [PATCH v5 2/6] reset: imx8mp-audiomix: Replace mask with bit index
Date: Tue, 25 Nov 2025 11:40:29 +0100	[thread overview]
Message-ID: <9f07e541fc000d9065c1ff1716f1edc4c2278c8d.camel@pengutronix.de> (raw)
In-Reply-To: <4a022153-009c-44fd-8c4b-39819ae69390@gmail.com>

On Di, 2025-11-25 at 01:59 -0800, Laurentiu Mihalcea wrote:
> On 11/24/2025 7:41 AM, Frank Li wrote:
> > On Mon, Nov 24, 2025 at 01:28:32AM -0800, Laurentiu Mihalcea wrote:
> > > On 11/21/2025 7:38 AM, Frank Li wrote:
> > > > On Fri, Nov 14, 2025 at 05:37:34AM -0800, Laurentiu Mihalcea wrote:
> > > > > From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> > > > > 
> > > > > Replace the reset map mask with the bit index to make it clear that all
> > > > > reset lines are managed by exactly 1 bit.
> > > > I don't think there are benefit because I met some periphal need a magic
> > > > number to reset.

Toggling multiple bits in unison is different from having to write a
magic number to a register field. The driver currently supports
neither. That is why I suggested to change from mask to bit.

> > > Please provide more information. What SoC? Which peripherals? What block control?
> > > 
> > I can't reminder exact one. I grep some code
> > 
> > [IMX8MP_MEDIABLK_PD_LCDIF_1] = {
> >                 .name = "mediablk-lcdif-1",
> >                 .clk_names = (const char *[]){ "disp1", "apb", "axi", },
> >                 .num_clks = 3,
> >                 .gpc_name = "lcdif1",
> >                 .rst_mask = BIT(4) | BIT(5) | BIT(23),
> >                 .clk_mask = BIT(4) | BIT(5) | BIT(23),

According to the reference manual, these are three separate software
resets for three separate clocks: lcdif_pixel_clk, lcdif_apb_clk, and
lcdif_axi_clk.

> >                 .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> >                 .num_paths = 2,
> >         },
> > 
> > mask is more extenable and easily support more hardware in future.

If such hardware appears in the future, it will be easy to adapt the
driver. Usually we don't prematurely add complexity for possible future
hardware.

> > Change to bit number have not big benefit.

It improves readability as it makes immediately clear from the code
that all resets correspond to a single bit.

> sure, I'm fine with the mask-based approach. The big idea here is to make this driver
> usable in as many scenarios as possible.
> 
> Philipp, please let me know if you're okay with the proposal. Will also have to tweak
> one of the subsequent patches since, so far, we've been operating under the assumption
> that reset lines are 1 bit.

Given that the current code is already using mask, and if you think it
is likely that there will be need for reset controls that require
toggling multiple bits with a single write, I'm fine with keeping the
mask.

regards
Philipp

  reply	other threads:[~2025-11-25 10:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 13:37 [PATCH v5 0/6] Add support for i.MX8ULP's SIM LPAV Laurentiu Mihalcea
2025-11-14 13:37 ` [PATCH v5 1/6] reset: imx8mp-audiomix: Drop unneeded macros Laurentiu Mihalcea
2025-11-14 15:48   ` Philipp Zabel
2025-11-14 13:37 ` [PATCH v5 2/6] reset: imx8mp-audiomix: Replace mask with bit index Laurentiu Mihalcea
2025-11-14 15:48   ` Philipp Zabel
2025-11-21 15:38   ` Frank Li
2025-11-24  9:28     ` Laurentiu Mihalcea
2025-11-24 15:41       ` Frank Li
2025-11-25  9:59         ` Laurentiu Mihalcea
2025-11-25 10:40           ` Philipp Zabel [this message]
2025-11-25 23:34             ` Laurentiu Mihalcea
2025-11-14 13:37 ` [PATCH v5 3/6] reset: imx8mp-audiomix: Switch to using regmap API Laurentiu Mihalcea
2025-11-14 15:49   ` Philipp Zabel
2025-11-21 15:43   ` Frank Li
2025-11-14 13:37 ` [PATCH v5 4/6] reset: imx8mp-audiomix: Extend the driver usage Laurentiu Mihalcea
2025-11-14 16:02   ` Philipp Zabel
2025-11-21  9:29     ` Laurentiu Mihalcea
2025-11-21 15:18       ` Philipp Zabel
2025-11-14 13:37 ` [PATCH v5 5/6] reset: imx8mp-audiomix: Support i.MX8ULP SIM LPAV Laurentiu Mihalcea
2025-11-14 16:09   ` Philipp Zabel
2025-11-14 13:37 ` [PATCH v5 6/6] arm64: dts: imx8ulp: add sim lpav node Laurentiu Mihalcea

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=9f07e541fc000d9065c1ff1716f1edc4c2278c8d.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=Frank.li@nxp.com \
    --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-kernel@vger.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).