From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
which allow users to control the assertion and de-assertion of the
reset lines tied to some peripherals present in said subsystems. Some
examples of such SoCs include i.MX8MP with AUDIOMIX block control and
i.MX8ULP with SIM LPAV.
Some of the aformentioned block control IPs exhibit a common pattern with
respect to the assertion and de-assertion of the reset lines. Namely, the
user is able to control the state of the reset line by toggling a bit from
one of the IP's registers.
Linux can take advantage of this pattern and, instead of having one driver
for each block control IP, a single, more generic driver could be used.
To allow this to happen, the previous approach, in which a single reset
map is used, is replaced by a per-driver approach, in which each auxiliary
device driver holds a reference to a certain reset map.
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
---
drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
index c74ce6e04177..c370913107f5 100644
--- a/drivers/reset/reset-imx8mp-audiomix.c
+++ b/drivers/reset/reset-imx8mp-audiomix.c
@@ -24,7 +24,12 @@ struct imx8mp_reset_map {
bool active_low;
};
-static const struct imx8mp_reset_map reset_map[] = {
+struct imx8mp_reset_info {
+ const struct imx8mp_reset_map *map;
+ int num_lines;
+};
+
+static const struct imx8mp_reset_map imx8mp_reset_map[] = {
[IMX8MP_AUDIOMIX_EARC_RESET] = {
.offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
.mask = BIT(0),
@@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
},
};
+static const struct imx8mp_reset_info imx8mp_reset_info = {
+ .map = imx8mp_reset_map,
+ .num_lines = ARRAY_SIZE(imx8mp_reset_map),
+};
+
struct imx8mp_audiomix_reset {
struct reset_controller_dev rcdev;
void __iomem *base;
struct regmap *regmap;
+ const struct imx8mp_reset_info *rinfo;
};
static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
@@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
unsigned long id, bool assert)
{
struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
+ const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
unsigned int mask, offset, active_low, shift, val;
mask = reset_map[id].mask;
@@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
return -ENOMEM;
priv->rcdev.owner = THIS_MODULE;
- priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
+ priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
+ priv->rcdev.nr_resets = priv->rinfo->num_lines;
priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
priv->rcdev.of_node = dev->parent->of_node;
priv->rcdev.dev = dev;
@@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
{
.name = "clk_imx8mp_audiomix.reset",
+ .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
},
{ }
};
--
2.43.0On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> which allow users to control the assertion and de-assertion of the
> reset lines tied to some peripherals present in said subsystems. Some
> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> i.MX8ULP with SIM LPAV.
>
> Some of the aformentioned block control IPs exhibit a common pattern with
> respect to the assertion and de-assertion of the reset lines. Namely, the
> user is able to control the state of the reset line by toggling a bit from
> one of the IP's registers.
>
> Linux can take advantage of this pattern and, instead of having one driver
> for each block control IP, a single, more generic driver could be used.
>
> To allow this to happen, the previous approach, in which a single reset
> map is used, is replaced by a per-driver approach, in which each auxiliary
> device driver holds a reference to a certain reset map.
Can you shorter your commit message?, basically, you just add
imx8mp_reset_info for difference auxiliary_device_id.
>
> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> ---
> drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> index c74ce6e04177..c370913107f5 100644
> --- a/drivers/reset/reset-imx8mp-audiomix.c
> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
> bool active_low;
> };
>
> -static const struct imx8mp_reset_map reset_map[] = {
> +struct imx8mp_reset_info {
> + const struct imx8mp_reset_map *map;
> + int num_lines;
> +};
> +
> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
> [IMX8MP_AUDIOMIX_EARC_RESET] = {
> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> .mask = BIT(0),
> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
> },
> };
>
> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> + .map = imx8mp_reset_map,
> + .num_lines = ARRAY_SIZE(imx8mp_reset_map),
> +};
> +
> struct imx8mp_audiomix_reset {
> struct reset_controller_dev rcdev;
> void __iomem *base;
> struct regmap *regmap;
> + const struct imx8mp_reset_info *rinfo;
> };
>
> static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> unsigned long id, bool assert)
> {
> struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> + const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
> unsigned int mask, offset, active_low, shift, val;
>
> mask = reset_map[id].mask;
> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> return -ENOMEM;
>
> priv->rcdev.owner = THIS_MODULE;
> - priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> + priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
needn't force convert from void*
Frank
> + priv->rcdev.nr_resets = priv->rinfo->num_lines;
> priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
> priv->rcdev.of_node = dev->parent->of_node;
> priv->rcdev.dev = dev;
> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> {
> .name = "clk_imx8mp_audiomix.reset",
> + .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> },
> { }
> };
> --
> 2.43.0
>
On Fri, Oct 17, 2025 at 5:57 PM Frank Li <Frank.li@nxp.com> wrote: > > On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote: > > From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> > > > > Some NXP SoCs integrate one or more, per-subsystem, block control IPs, > > which allow users to control the assertion and de-assertion of the > > reset lines tied to some peripherals present in said subsystems. Some > > examples of such SoCs include i.MX8MP with AUDIOMIX block control and > > i.MX8ULP with SIM LPAV. > > > > Some of the aformentioned block control IPs exhibit a common pattern with > > respect to the assertion and de-assertion of the reset lines. Namely, the > > user is able to control the state of the reset line by toggling a bit from > > one of the IP's registers. > > > > Linux can take advantage of this pattern and, instead of having one driver > > for each block control IP, a single, more generic driver could be used. > > > > To allow this to happen, the previous approach, in which a single reset > > map is used, is replaced by a per-driver approach, in which each auxiliary > > device driver holds a reference to a certain reset map. > > Can you shorter your commit message?, basically, you just add > imx8mp_reset_info for difference auxiliary_device_id. It is always preferred to have commit messages trying to explain the context. I have never heard anyone complaining that the commit message is too long :D. Daniel.
On 10/17/2025 7:56 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
>> which allow users to control the assertion and de-assertion of the
>> reset lines tied to some peripherals present in said subsystems. Some
>> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
>> i.MX8ULP with SIM LPAV.
>>
>> Some of the aformentioned block control IPs exhibit a common pattern with
>> respect to the assertion and de-assertion of the reset lines. Namely, the
>> user is able to control the state of the reset line by toggling a bit from
>> one of the IP's registers.
>>
>> Linux can take advantage of this pattern and, instead of having one driver
>> for each block control IP, a single, more generic driver could be used.
>>
>> To allow this to happen, the previous approach, in which a single reset
>> map is used, is replaced by a per-driver approach, in which each auxiliary
>> device driver holds a reference to a certain reset map.
> Can you shorter your commit message?, basically, you just add
> imx8mp_reset_info for difference auxiliary_device_id.
>
>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>> drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index c74ce6e04177..c370913107f5 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
>> bool active_low;
>> };
>>
>> -static const struct imx8mp_reset_map reset_map[] = {
>> +struct imx8mp_reset_info {
>> + const struct imx8mp_reset_map *map;
>> + int num_lines;
>> +};
>> +
>> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
>> [IMX8MP_AUDIOMIX_EARC_RESET] = {
>> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>> .mask = BIT(0),
>> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
>> },
>> };
>>
>> +static const struct imx8mp_reset_info imx8mp_reset_info = {
>> + .map = imx8mp_reset_map,
>> + .num_lines = ARRAY_SIZE(imx8mp_reset_map),
>> +};
>> +
>> struct imx8mp_audiomix_reset {
>> struct reset_controller_dev rcdev;
>> void __iomem *base;
>> struct regmap *regmap;
>> + const struct imx8mp_reset_info *rinfo;
>> };
>>
>> static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
>> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>> unsigned long id, bool assert)
>> {
>> struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>> + const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
>> unsigned int mask, offset, active_low, shift, val;
>>
>> mask = reset_map[id].mask;
>> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>> return -ENOMEM;
>>
>> priv->rcdev.owner = THIS_MODULE;
>> - priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>> + priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
> needn't force convert from void*
not a void *, "id->driver_data" is kernel_ulong_t
>
> Frank
>
>> + priv->rcdev.nr_resets = priv->rinfo->num_lines;
>> priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
>> priv->rcdev.of_node = dev->parent->of_node;
>> priv->rcdev.dev = dev;
>> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>> static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>> {
>> .name = "clk_imx8mp_audiomix.reset",
>> + .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>> },
>> { }
>> };
>> --
>> 2.43.0
>>
On Mon, Oct 20, 2025 at 07:06:55AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/17/2025 7:56 AM, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> >> which allow users to control the assertion and de-assertion of the
> >> reset lines tied to some peripherals present in said subsystems. Some
> >> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> >> i.MX8ULP with SIM LPAV.
> >>
> >> Some of the aformentioned block control IPs exhibit a common pattern with
> >> respect to the assertion and de-assertion of the reset lines. Namely, the
> >> user is able to control the state of the reset line by toggling a bit from
> >> one of the IP's registers.
> >>
> >> Linux can take advantage of this pattern and, instead of having one driver
> >> for each block control IP, a single, more generic driver could be used.
> >>
> >> To allow this to happen, the previous approach, in which a single reset
> >> map is used, is replaced by a per-driver approach, in which each auxiliary
> >> device driver holds a reference to a certain reset map.
> > Can you shorter your commit message?, basically, you just add
> > imx8mp_reset_info for difference auxiliary_device_id.
> >
> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >> drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
> >> 1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >> index c74ce6e04177..c370913107f5 100644
> >> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
> >> bool active_low;
> >> };
> >>
> >> -static const struct imx8mp_reset_map reset_map[] = {
> >> +struct imx8mp_reset_info {
> >> + const struct imx8mp_reset_map *map;
> >> + int num_lines;
> >> +};
> >> +
> >> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
> >> [IMX8MP_AUDIOMIX_EARC_RESET] = {
> >> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> >> .mask = BIT(0),
> >> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
> >> },
> >> };
> >>
> >> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> >> + .map = imx8mp_reset_map,
> >> + .num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >> +};
> >> +
> >> struct imx8mp_audiomix_reset {
> >> struct reset_controller_dev rcdev;
> >> void __iomem *base;
> >> struct regmap *regmap;
> >> + const struct imx8mp_reset_info *rinfo;
> >> };
> >>
> >> static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> >> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> >> unsigned long id, bool assert)
> >> {
> >> struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> >> + const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
> >> unsigned int mask, offset, active_low, shift, val;
> >>
> >> mask = reset_map[id].mask;
> >> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >> return -ENOMEM;
> >>
> >> priv->rcdev.owner = THIS_MODULE;
> >> - priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> >> + priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
> > needn't force convert from void*
>
>
> not a void *, "id->driver_data" is kernel_ulong_t
why not use void *?
Frank
>
>
> >
> > Frank
> >
> >> + priv->rcdev.nr_resets = priv->rinfo->num_lines;
> >> priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
> >> priv->rcdev.of_node = dev->parent->of_node;
> >> priv->rcdev.dev = dev;
> >> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >> static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >> {
> >> .name = "clk_imx8mp_audiomix.reset",
> >> + .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >> },
> >> { }
> >> };
> >> --
> >> 2.43.0
> >>
On 10/17/2025 7:56 AM, Frank Li wrote:
> On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
>> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>>
>> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
>> which allow users to control the assertion and de-assertion of the
>> reset lines tied to some peripherals present in said subsystems. Some
>> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
>> i.MX8ULP with SIM LPAV.
>>
>> Some of the aformentioned block control IPs exhibit a common pattern with
>> respect to the assertion and de-assertion of the reset lines. Namely, the
>> user is able to control the state of the reset line by toggling a bit from
>> one of the IP's registers.
>>
>> Linux can take advantage of this pattern and, instead of having one driver
>> for each block control IP, a single, more generic driver could be used.
>>
>> To allow this to happen, the previous approach, in which a single reset
>> map is used, is replaced by a per-driver approach, in which each auxiliary
>> device driver holds a reference to a certain reset map.
> Can you shorter your commit message?, basically, you just add
> imx8mp_reset_info for difference auxiliary_device_id.
yeah, but the commit message is not trying to explain the code, but, rather,
offer a bit more context to understand WHY we're doing this. However, I'm not
fixated on this so if people don't think it's useful then I'll just shorten it.
>
>> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
>> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>> ---
>> drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
>> index c74ce6e04177..c370913107f5 100644
>> --- a/drivers/reset/reset-imx8mp-audiomix.c
>> +++ b/drivers/reset/reset-imx8mp-audiomix.c
>> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
>> bool active_low;
>> };
>>
>> -static const struct imx8mp_reset_map reset_map[] = {
>> +struct imx8mp_reset_info {
>> + const struct imx8mp_reset_map *map;
>> + int num_lines;
>> +};
>> +
>> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
>> [IMX8MP_AUDIOMIX_EARC_RESET] = {
>> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
>> .mask = BIT(0),
>> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
>> },
>> };
>>
>> +static const struct imx8mp_reset_info imx8mp_reset_info = {
>> + .map = imx8mp_reset_map,
>> + .num_lines = ARRAY_SIZE(imx8mp_reset_map),
>> +};
>> +
>> struct imx8mp_audiomix_reset {
>> struct reset_controller_dev rcdev;
>> void __iomem *base;
>> struct regmap *regmap;
>> + const struct imx8mp_reset_info *rinfo;
>> };
>>
>> static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
>> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
>> unsigned long id, bool assert)
>> {
>> struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
>> + const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
>> unsigned int mask, offset, active_low, shift, val;
>>
>> mask = reset_map[id].mask;
>> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>> return -ENOMEM;
>>
>> priv->rcdev.owner = THIS_MODULE;
>> - priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
>> + priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
> needn't force convert from void*
>
> Frank
>
>> + priv->rcdev.nr_resets = priv->rinfo->num_lines;
>> priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
>> priv->rcdev.of_node = dev->parent->of_node;
>> priv->rcdev.dev = dev;
>> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
>> static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
>> {
>> .name = "clk_imx8mp_audiomix.reset",
>> + .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
>> },
>> { }
>> };
>> --
>> 2.43.0
>>
On Mon, Oct 20, 2025 at 04:59:52AM -0700, Laurentiu Mihalcea wrote:
>
> On 10/17/2025 7:56 AM, Frank Li wrote:
> > On Fri, Oct 17, 2025 at 04:20:23AM -0700, Laurentiu Mihalcea wrote:
> >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >>
> >> Some NXP SoCs integrate one or more, per-subsystem, block control IPs,
> >> which allow users to control the assertion and de-assertion of the
> >> reset lines tied to some peripherals present in said subsystems. Some
> >> examples of such SoCs include i.MX8MP with AUDIOMIX block control and
> >> i.MX8ULP with SIM LPAV.
> >>
> >> Some of the aformentioned block control IPs exhibit a common pattern with
> >> respect to the assertion and de-assertion of the reset lines. Namely, the
> >> user is able to control the state of the reset line by toggling a bit from
> >> one of the IP's registers.
> >>
> >> Linux can take advantage of this pattern and, instead of having one driver
> >> for each block control IP, a single, more generic driver could be used.
> >>
> >> To allow this to happen, the previous approach, in which a single reset
> >> map is used, is replaced by a per-driver approach, in which each auxiliary
> >> device driver holds a reference to a certain reset map.
> > Can you shorter your commit message?, basically, you just add
> > imx8mp_reset_info for difference auxiliary_device_id.
>
>
> yeah, but the commit message is not trying to explain the code, but, rather,
>
> offer a bit more context to understand WHY we're doing this. However, I'm not
>
> fixated on this so if people don't think it's useful then I'll just shorten it.
Not necessary to mention obvious facts. A commit message should summarize
meaningful information — it’s meant to highlight what’s important, not what
everyone already knows.
Using driver_data to distinguish between different chips or blocks is a
common and well-established approach used by many drivers.
For example: below commit message is enough to bring necesary informaiton
to reviewer.
"Managing peripheral reset lines by toggling a register bit is widely used
on NXP SoCs such as i.MX8MP and i.MX8ULP.
Add imx8mp_reset_map to the auxiliary_device_id's driver data to allow
reuse of the same driver for different reset controller blocks."
Frank
>
>
> >
> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
> >> ---
> >> drivers/reset/reset-imx8mp-audiomix.c | 18 ++++++++++++++++--
> >> 1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/reset/reset-imx8mp-audiomix.c b/drivers/reset/reset-imx8mp-audiomix.c
> >> index c74ce6e04177..c370913107f5 100644
> >> --- a/drivers/reset/reset-imx8mp-audiomix.c
> >> +++ b/drivers/reset/reset-imx8mp-audiomix.c
> >> @@ -24,7 +24,12 @@ struct imx8mp_reset_map {
> >> bool active_low;
> >> };
> >>
> >> -static const struct imx8mp_reset_map reset_map[] = {
> >> +struct imx8mp_reset_info {
> >> + const struct imx8mp_reset_map *map;
> >> + int num_lines;
> >> +};
> >> +
> >> +static const struct imx8mp_reset_map imx8mp_reset_map[] = {
> >> [IMX8MP_AUDIOMIX_EARC_RESET] = {
> >> .offset = IMX8MP_AUDIOMIX_EARC_RESET_OFFSET,
> >> .mask = BIT(0),
> >> @@ -45,10 +50,16 @@ static const struct imx8mp_reset_map reset_map[] = {
> >> },
> >> };
> >>
> >> +static const struct imx8mp_reset_info imx8mp_reset_info = {
> >> + .map = imx8mp_reset_map,
> >> + .num_lines = ARRAY_SIZE(imx8mp_reset_map),
> >> +};
> >> +
> >> struct imx8mp_audiomix_reset {
> >> struct reset_controller_dev rcdev;
> >> void __iomem *base;
> >> struct regmap *regmap;
> >> + const struct imx8mp_reset_info *rinfo;
> >> };
> >>
> >> static struct imx8mp_audiomix_reset *to_imx8mp_audiomix_reset(struct reset_controller_dev *rcdev)
> >> @@ -60,6 +71,7 @@ static int imx8mp_audiomix_update(struct reset_controller_dev *rcdev,
> >> unsigned long id, bool assert)
> >> {
> >> struct imx8mp_audiomix_reset *priv = to_imx8mp_audiomix_reset(rcdev);
> >> + const struct imx8mp_reset_map *reset_map = priv->rinfo->map;
> >> unsigned int mask, offset, active_low, shift, val;
> >>
> >> mask = reset_map[id].mask;
> >> @@ -144,7 +156,8 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >> return -ENOMEM;
> >>
> >> priv->rcdev.owner = THIS_MODULE;
> >> - priv->rcdev.nr_resets = ARRAY_SIZE(reset_map);
> >> + priv->rinfo = (const struct imx8mp_reset_info *)id->driver_data;
> > needn't force convert from void*
> >
> > Frank
> >
> >> + priv->rcdev.nr_resets = priv->rinfo->num_lines;
> >> priv->rcdev.ops = &imx8mp_audiomix_reset_ops;
> >> priv->rcdev.of_node = dev->parent->of_node;
> >> priv->rcdev.dev = dev;
> >> @@ -168,6 +181,7 @@ static int imx8mp_audiomix_reset_probe(struct auxiliary_device *adev,
> >> static const struct auxiliary_device_id imx8mp_audiomix_reset_ids[] = {
> >> {
> >> .name = "clk_imx8mp_audiomix.reset",
> >> + .driver_data = (kernel_ulong_t)&imx8mp_reset_info,
> >> },
> >> { }
> >> };
> >> --
> >> 2.43.0
> >>
© 2016 - 2026 Red Hat, Inc.