Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201207085931.661267-1-allen.lkml@gmail.com> <20201207085931.661267-8-allen.lkml@gmail.com>
In-Reply-To: <20201207085931.661267-8-allen.lkml@gmail.com>
From:   Gilad Ben-Yossef <gilad@benyossef.com>
Date:   Wed, 9 Dec 2020 08:46:47 +0200
Message-ID: <CAOtvUMeAQYwoB_9jmMdwi8tTYtD8=-r5T7RFTiKgEnDHgkbP+g@mail.gmail.com>
Subject: Re: [RESEND 07/19] crypto: ccree: convert tasklets to use new
 tasklet_setup() API
To:     Allen Pais <allen.lkml@gmail.com>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        David Miller <davem@davemloft.net>,
        nicolas.ferre@microchip.com, alexandre.belloni@bootlin.com,
        ludovic.desroches@microchip.com, jesper.nilsson@axis.com,
        lars.persson@axis.com,
        =?UTF-8?Q?Horia_Geant=C4=83?= <horia.geanta@nxp.com>,
        aymen.sghaier@nxp.com, bbrezillon@kernel.org,
        Arnaud Ebalard <arno@natisbad.org>, schalla@marvell.com,
        Matthias Brugger <matthias.bgg@gmail.com>, heiko@sntech.de,
        krzk@kernel.org, vz@mleia.com, k.konieczny@samsung.com,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Allen Pais <apais@microsoft.com>,
        Romain Perier <romain.perier@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Allen,

Thank you for the patch.

Please make sure to CC maintainers on changes to drivers they
maintain, otherwise it's hard to keep track. Thanks!

On Mon, Dec 7, 2020 at 11:02 AM Allen Pais <allen.lkml@gmail.com> wrote:
>
> From: Allen Pais <apais@microsoft.com>
>
> In preparation for unconditionally passing the
> struct tasklet_struct pointer to all tasklet
> callbacks, switch to using the new tasklet_setup()
> and from_tasklet() to pass the tasklet pointer explicitly.
>
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> Signed-off-by: Allen Pais <apais@microsoft.com>
> ---
>  drivers/crypto/ccree/cc_fips.c        |  6 +++---
>  drivers/crypto/ccree/cc_request_mgr.c | 12 ++++++------
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/crypto/ccree/cc_fips.c b/drivers/crypto/ccree/cc_fip=
s.c
> index 702aefc21447..bad228a36776 100644
> --- a/drivers/crypto/ccree/cc_fips.c
> +++ b/drivers/crypto/ccree/cc_fips.c
> @@ -109,9 +109,9 @@ void cc_tee_handle_fips_error(struct cc_drvdata *p_dr=
vdata)
>  }
>
>  /* Deferred service handler, run as interrupt-fired tasklet */
> -static void fips_dsr(unsigned long devarg)
> +static void fips_dsr(struct tasklet_struct *t)

Sorry for the nitpick, but I would really prefer to have a more
meaningful name for this parameter than just 't'.

tasklet, task, tsk... any descriptive name is fine.

>  {
> -       struct cc_drvdata *drvdata =3D (struct cc_drvdata *)devarg;
> +       struct cc_drvdata *drvdata =3D from_tasklet(drvdata, t, tasklet);
>         u32 irq, val;
>
>         irq =3D (drvdata->irq & (CC_GPR0_IRQ_MASK));
> @@ -143,7 +143,7 @@ int cc_fips_init(struct cc_drvdata *p_drvdata)
>         p_drvdata->fips_handle =3D fips_h;
>
>         dev_dbg(dev, "Initializing fips tasklet\n");
> -       tasklet_init(&fips_h->tasklet, fips_dsr, (unsigned long)p_drvdata=
);
> +       tasklet_setup(&fips_h->tasklet, fips_dsr);
>         fips_h->drvdata =3D p_drvdata;
>         fips_h->nb.notifier_call =3D cc_ree_fips_failure;
>         atomic_notifier_chain_register(&fips_fail_notif_chain, &fips_h->n=
b);
> diff --git a/drivers/crypto/ccree/cc_request_mgr.c b/drivers/crypto/ccree=
/cc_request_mgr.c
> index 33fb27745d52..ec0f3bf00d33 100644
> --- a/drivers/crypto/ccree/cc_request_mgr.c
> +++ b/drivers/crypto/ccree/cc_request_mgr.c
> @@ -70,7 +70,7 @@ static const u32 cc_cpp_int_masks[CC_CPP_NUM_ALGS][CC_C=
PP_NUM_SLOTS] =3D {
>           BIT(CC_HOST_IRR_REE_OP_ABORTED_SM_7_INT_BIT_SHIFT) }
>  };
>
> -static void comp_handler(unsigned long devarg);
> +static void comp_handler(struct tasklet_struct *t);
>  #ifdef COMP_IN_WQ
>  static void comp_work_handler(struct work_struct *work);
>  #endif
> @@ -140,8 +140,7 @@ int cc_req_mgr_init(struct cc_drvdata *drvdata)
>         INIT_DELAYED_WORK(&req_mgr_h->compwork, comp_work_handler);
>  #else
>         dev_dbg(dev, "Initializing completion tasklet\n");
> -       tasklet_init(&req_mgr_h->comptask, comp_handler,
> -                    (unsigned long)drvdata);
> +       tasklet_setup(&req_mgr_h->comptask, comp_handler);
>  #endif
>         req_mgr_h->hw_queue_size =3D cc_ioread(drvdata,
>                                              CC_REG(DSCRPTR_QUEUE_SRAM_SI=
ZE));
> @@ -611,11 +610,12 @@ static inline u32 cc_axi_comp_count(struct cc_drvda=
ta *drvdata)
>  }
>
>  /* Deferred service handler, run as interrupt-fired tasklet */
> -static void comp_handler(unsigned long devarg)
> +static void comp_handler(struct tasklet_struct *t)
>  {
> -       struct cc_drvdata *drvdata =3D (struct cc_drvdata *)devarg;
>         struct cc_req_mgr_handle *request_mgr_handle =3D
> -                                               drvdata->request_mgr_hand=
le;
> +                               from_tasklet(request_mgr_handle, t, compt=
ask);
> +       struct cc_drvdata *drvdata =3D container_of((void *)request_mgr_h=
andle,
> +                                    typeof(*drvdata), request_mgr_handle=
);
>         struct device *dev =3D drvdata_to_dev(drvdata);
>         u32 irq;
>
> --
> 2.25.1
>

Other than that it looks good to me.

Thanks,
Gilad

--=20
Gilad Ben-Yossef
Chief Coffee Drinker

values of =CE=B2 will give rise to dom!