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.
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
drivers/reset/reset-imx8mp-audiomix.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index acfa92b15329..d993c294c548 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -19,24 +19,24 @@
struct imx8mp_reset_map {
unsigned int offset;
- unsigned int mask;
+ unsigned int bit;
bool active_low;
};
static const struct imx8mp_reset_map reset_map[] = {
[IMX8MP_AUDIOMIX_EARC_RESET] = {
.offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
- .mask = BIT(0),
+ .bit = 0,
.active_low = true,
},
[IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
.offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
- .mask = BIT(1),
+ .bit = 1,
.active_low = true,
},
[IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
.offset = IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
- .mask = BIT(5),
+ .bit = 5,
.active_low = false,
},
};
@@ -60,7 +60,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
unsigned int mask, offset, active_low;
unsigned long reg, flags;
- mask = reset_map[id].mask;
+ mask = BIT(reset_map[id].bit);
offset = reset_map[id].offset;
active_low = reset_map[id].active_low;
--
2.43.0On 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.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
> drivers/reset/reset-imx8mp-audiomix.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index acfa92b15329..d993c294c548 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -19,24 +19,24 @@
>
> struct imx8mp_reset_map {
> unsigned int offset;
> - unsigned int mask;
> + unsigned int bit;
> bool active_low;
> };
>
> static const struct imx8mp_reset_map reset_map[] = {
> [IMX8MP_AUDIOMIX_EARC_RESET] = {
> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> - .mask = BIT(0),
> + .bit = 0,
> .active_low = true,
> },
> [IMX8MP_AUDIOMIX_EARC_PHY_RESET] = {
> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> - .mask = BIT(1),
> + .bit = 1,
> .active_low = true,
> },
> [IMX8MP_AUDIOMIX_DSP_RUNSTALL] = {
> .offset = IMX8MP_AUDIOMIX_DSP_RUNSTALL_OFFSET,
> - .mask = BIT(5),
> + .bit = 5,
> .active_low = false,
> },
> };
> @@ -60,7 +60,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> unsigned int mask, offset, active_low;
> unsigned long reg, flags;
>
> - mask = reset_map[id].mask;
> + mask = BIT(reset_map[id].bit);
> offset = reset_map[id].offset;
> active_low = reset_map[id].active_low;
>
> --
> 2.43.0
>
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. Please provide more information. What SoC? Which peripherals? What block control?
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.
>
>
> 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),
.path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
.num_paths = 2,
},
mask is more extenable and easily support more hardware in future. Change
to bit number have not big benefit.
Frank
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.
>>
>> 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),
> .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"},
> .num_paths = 2,
> },
>
> mask is more extenable and easily support more hardware in future. Change
> to bit number have not big benefit.
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.
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
On 11/25/2025 12:40 PM, Philipp Zabel wrote:
> 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.
ACK. Unfortunately, I don't have an use-case for this driver in which we'd want to
manage multiple underlying reset lines as a single one like in Frank's example. I'm
also assuming Frank doesn't have one either based on his previous comment.
Despite this, however, the previous version of the driver was already able to handle
this use-case. The single-bit reset restriction was introduced by this series. Therefore, I
suggest we reduce the number of changes and revert to the old behavior.
This way, we'll avoid reverting the patches if someone does ever come up with
such an use-case. I'd say the driver is simple enough that the code readability will
not be severely impacted.
Either way, I think we'd be fine with any of the two approaches in the end.
>
> regards
> Philipp
On Fr, 2025-11-14 at 05:37 -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. > > Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
© 2016 - 2026 Red Hat, Inc.