[v2,2/5] baseband/acc100: modify validation code for ACC101

Message ID 1651083423-33202-3-git-send-email-nicolas.chautru@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series drivers/baseband: PMD to support ACC101 device |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chautru, Nicolas April 27, 2022, 6:17 p.m. UTC
  The validation requirement is different for the two
devices.

Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
---
 drivers/baseband/acc100/rte_acc100_pmd.c | 40 ++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 12 deletions(-)
  

Comments

Tom Rix May 8, 2022, 1:07 p.m. UTC | #1
On 4/27/22 11:17 AM, Nicolas Chautru wrote:
> The validation requirement is different for the two
> devices.
>
> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> ---
>   drivers/baseband/acc100/rte_acc100_pmd.c | 40 ++++++++++++++++++++++----------
>   1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
> index fca27ef..daf2ce0 100644
> --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> @@ -1293,6 +1293,14 @@
>   	return (q->d->device_variant == ACC100_VARIANT);
>   }
>   
> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> +static inline bool
> +validate_op_required(struct acc100_queue *q)

There isn't an #else case so this will fail to build.

This i believe could be another function in private data fops i 
suggested in the first patch.

Tom

> +{
> +	return is_acc100(q);
> +}
> +#endif
> +
  
Chautru, Nicolas May 9, 2022, 9:27 p.m. UTC | #2
Hi Tom,

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Sunday, May 8, 2022 6:07 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> gakhil@marvell.com
> Cc: thomas@monjalon.net; Kinsella, Ray <ray.kinsella@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Zhang,
> Mingshan <mingshan.zhang@intel.com>; david.marchand@redhat.com
> Subject: Re: [PATCH v2 2/5] baseband/acc100: modify validation code for
> ACC101
> 
> 
> On 4/27/22 11:17 AM, Nicolas Chautru wrote:
> > The validation requirement is different for the two devices.
> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > ---
> >   drivers/baseband/acc100/rte_acc100_pmd.c | 40
> ++++++++++++++++++++++----------
> >   1 file changed, 28 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index fca27ef..daf2ce0 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -1293,6 +1293,14 @@
> >   	return (q->d->device_variant == ACC100_VARIANT);
> >   }
> >
> > +#ifdef RTE_LIBRTE_BBDEV_DEBUG
> > +static inline bool
> > +validate_op_required(struct acc100_queue *q)
> 
> There isn't an #else case so this will fail to build.

There is no else required I believe, since that function is not used when the RTE_LIBRTE_BBDEV_DEBUG is not set. It should build on both (I believe this is tested by CICD). 

> 
> This i believe could be another function in private data fops i suggested in the
> first patch.

In that case we do not expect to validate the input on ACC100, there is not such function. 

> 
> Tom
> 
> > +{
> > +	return is_acc100(q);
> > +}
> > +#endif
> > +
  

Patch

diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c b/drivers/baseband/acc100/rte_acc100_pmd.c
index fca27ef..daf2ce0 100644
--- a/drivers/baseband/acc100/rte_acc100_pmd.c
+++ b/drivers/baseband/acc100/rte_acc100_pmd.c
@@ -1293,6 +1293,14 @@ 
 	return (q->d->device_variant == ACC100_VARIANT);
 }
 
+#ifdef RTE_LIBRTE_BBDEV_DEBUG
+static inline bool
+validate_op_required(struct acc100_queue *q)
+{
+	return is_acc100(q);
+}
+#endif
+
 /* Fill in a frame control word for LDPC decoding. */
 static inline void
 acc100_fcw_ld_fill(const struct rte_bbdev_dec_op *op, struct acc100_fcw_ld *fcw,
@@ -2187,8 +2195,10 @@  static inline uint32_t hq_index(uint32_t offset)
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 /* Validates turbo encoder parameters */
 static inline int
-validate_enc_op(struct rte_bbdev_enc_op *op)
+validate_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_turbo_enc *turbo_enc = &op->turbo_enc;
 	struct rte_bbdev_op_enc_turbo_cb_params *cb = NULL;
 	struct rte_bbdev_op_enc_turbo_tb_params *tb = NULL;
@@ -2325,8 +2335,10 @@  static inline uint32_t hq_index(uint32_t offset)
 }
 /* Validates LDPC encoder parameters */
 static inline int
-validate_ldpc_enc_op(struct rte_bbdev_enc_op *op)
+validate_ldpc_enc_op(struct rte_bbdev_enc_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_ldpc_enc *ldpc_enc = &op->ldpc_enc;
 
 	if (op->mempool == NULL) {
@@ -2378,8 +2390,10 @@  static inline uint32_t hq_index(uint32_t offset)
 
 /* Validates LDPC decoder parameters */
 static inline int
-validate_ldpc_dec_op(struct rte_bbdev_dec_op *op)
+validate_ldpc_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_ldpc_dec *ldpc_dec = &op->ldpc_dec;
 
 	if (op->mempool == NULL) {
@@ -2434,7 +2448,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_enc_op(op) == -1) {
+	if (validate_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo encoder validation failed");
 		return -EINVAL;
 	}
@@ -2488,7 +2502,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_enc_op(ops[0]) == -1) {
+	if (validate_ldpc_enc_op(ops[0], q) == -1) {
 		rte_bbdev_log(ERR, "LDPC encoder validation failed");
 		return -EINVAL;
 	}
@@ -2550,7 +2564,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_enc_op(op) == -1) {
+	if (validate_ldpc_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC encoder validation failed");
 		return -EINVAL;
 	}
@@ -2607,7 +2621,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_enc_op(op) == -1) {
+	if (validate_enc_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo encoder validation failed");
 		return -EINVAL;
 	}
@@ -2680,8 +2694,10 @@  static inline uint32_t hq_index(uint32_t offset)
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 /* Validates turbo decoder parameters */
 static inline int
-validate_dec_op(struct rte_bbdev_dec_op *op)
+validate_dec_op(struct rte_bbdev_dec_op *op, struct acc100_queue *q)
 {
+	if (!validate_op_required(q))
+		return 0;
 	struct rte_bbdev_op_turbo_dec *turbo_dec = &op->turbo_dec;
 	struct rte_bbdev_op_dec_turbo_cb_params *cb = NULL;
 	struct rte_bbdev_op_dec_turbo_tb_params *tb = NULL;
@@ -2827,7 +2843,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_dec_op(op) == -1) {
+	if (validate_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo decoder validation failed");
 		return -EINVAL;
 	}
@@ -3052,7 +3068,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_dec_op(op) == -1) {
+	if (validate_ldpc_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC decoder validation failed");
 		return -EINVAL;
 	}
@@ -3156,7 +3172,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_ldpc_dec_op(op) == -1) {
+	if (validate_ldpc_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "LDPC decoder validation failed");
 		return -EINVAL;
 	}
@@ -3246,7 +3262,7 @@  static inline uint32_t hq_index(uint32_t offset)
 
 #ifdef RTE_LIBRTE_BBDEV_DEBUG
 	/* Validate op structure */
-	if (validate_dec_op(op) == -1) {
+	if (validate_dec_op(op, q) == -1) {
 		rte_bbdev_log(ERR, "Turbo decoder validation failed");
 		return -EINVAL;
 	}