On Mon, 25 Mar 2024 11:40:20 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> On 3/23/24 18:43, Marek Behún wrote:
> > Add resource managed version of irq_create_mapping(), to help drivers
> > automatically dispose a linux irq mapping when driver is detached.
> >
> > The new function devm_irq_create_mapping() is not yet used, but the
> > action function can be used in the FSL CAAM driver.
> >
> > Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> > ---
> > drivers/crypto/caam/jr.c | 8 ++----
> > include/linux/devm-helpers.h | 54 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 56 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> > index 26eba7de3fb0..ad0295b055f8 100644
> > --- a/drivers/crypto/caam/jr.c
> > +++ b/drivers/crypto/caam/jr.c
> > @@ -7,6 +7,7 @@
> > * Copyright 2019, 2023 NXP
> > */
> >
> > +#include <linux/devm-helpers.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_address.h>
> > #include <linux/platform_device.h>
> > @@ -576,11 +577,6 @@ static int caam_jr_init(struct device *dev)
> > return error;
> > }
> >
> > -static void caam_jr_irq_dispose_mapping(void *data)
> > -{
> > - irq_dispose_mapping((unsigned long)data);
> > -}
> > -
> > /*
> > * Probe routine for each detected JobR subsystem.
> > */
> > @@ -656,7 +652,7 @@ static int caam_jr_probe(struct platform_device *pdev)
> > return -EINVAL;
> > }
> >
> > - error = devm_add_action_or_reset(jrdev, caam_jr_irq_dispose_mapping,
> > + error = devm_add_action_or_reset(jrdev, devm_irq_mapping_drop,
> > (void *)(unsigned long)jrpriv->irq);
> > if (error)
> > return error;
> > diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> > index 74891802200d..3805551fd433 100644
> > --- a/include/linux/devm-helpers.h
> > +++ b/include/linux/devm-helpers.h
> > @@ -24,6 +24,8 @@
> > */
> >
> > #include <linux/device.h>
> > +#include <linux/kconfig.h>
> > +#include <linux/irqdomain.h>
> > #include <linux/workqueue.h>
>
> My confidence level is not terribly high today, so I am likely to accept
> just about any counter arguments :) But ... More I think of this whole
> header, less convinced I am that this (the header) is a great idea. I
> wonder who has authored a concept like this... :rolleyes:
>
> Pulling punch of unrelated APIs (or, unrelated except the devm-usage) in
> one header has potential to be including a lot of unneeded stuff to the
> users. I am under impression this can be bad for example for the build
> times.
>
> I think that ideally the devm-APIs should live close to their non-devm
> counterparts, and this header should be just used as a last resort, when
> all the other options fail :) May I assume all other options have failed
> for the IRQ stuff?
>
> Well, I will leave the big picture to the bigger minds. When just
> looking at the important things like the function names and coding style
> - this change looks Ok to me ;)
If the authors of devm-helpers or someone else decide it should not
exist (due for example of long build times), I am OK with that.
But currently this seems to me to be the proper place to put this into.
Marek
