On Thu, Mar 09, 2023 at 06:39:45PM +0800, Herbert Xu wrote:
> On Thu, Mar 09, 2023 at 11:33:06AM +0000, Meadhbh Fitzpatrick wrote:
> >
> > diff --git a/drivers/crypto/qat/qat_common/qat_comp_algs.c b/drivers/crypto/qat/qat_common/qat_comp_algs.c
> > index b533984906ec..726b71d2a4a8 100644
> > --- a/drivers/crypto/qat/qat_common/qat_comp_algs.c
> > +++ b/drivers/crypto/qat/qat_common/qat_comp_algs.c
> > @@ -183,7 +183,7 @@ static void qat_comp_generic_callback(struct qat_compression_req *qat_req,
> > int consumed, produced;
> > s8 cmp_err, xlt_err;
> > int res = -EBADMSG;
> > - int status;
> > + s8 status;
>
> Sorry, but this makes no sense. Why are these s8's to begin
> with? It doesn't look like you even allow negative values.
You are right. Thanks for spotting this.
When reviewing it I mixed up the cmp_err and xlt_err with the status which
is a u8.
The functions qat_comp_get_cmp_status() and qat_comp_get_xlt_status()
should be changed to return an u8 as the field returned by the firmware
is an unsigned byte [1].
The `status` variable in qat_comp_generic_callback() can be changed to s8
or stay as int. It is just used to check if a value is set.
...
if (unlikely(status != ICP_QAT_FW_COMN_STATUS_FLAG_OK))
goto end;
...
where ICP_QAT_FW_COMN_STATUS_FLAG_OK is 0.
[1] https://elixir.bootlin.com/linux/v6.3-rc1/source/drivers/crypto/qat/qat_common/icp_qat_fw.h#L104
Regards,
--
Giovanni
