> -----Original Message----- > From: Ruxandra Ioana Radulescu > Sent: Thursday, November 10, 2016 9:04 AM > To: Stuart Yoder <stuart.yoder@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx > Cc: German Rivera <german.rivera@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > agraf@xxxxxxx; arnd@xxxxxxxx; Leo Li <leoyang.li@xxxxxxx>; Roy Pledge <roy.pledge@xxxxxxx>; Haiying Wang > <haiying.wang@xxxxxxx>; Stuart Yoder <stuart.yoder@xxxxxxx> > Subject: RE: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 > > > > -----Original Message----- > > From: Stuart Yoder [mailto:stuart.yoder@xxxxxxx] > > Sent: Friday, October 21, 2016 9:02 AM > > To: gregkh@xxxxxxxxxxxxxxxxxxx > > Cc: German Rivera <german.rivera@xxxxxxx>; devel@xxxxxxxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; agraf@xxxxxxx; arnd@xxxxxxxx; Leo Li > > <leoyang.li@xxxxxxx>; Roy Pledge <roy.pledge@xxxxxxx>; Roy Pledge > > <roy.pledge@xxxxxxx>; Haiying Wang <haiying.wang@xxxxxxx>; Stuart > > Yoder <stuart.yoder@xxxxxxx> > > Subject: [PATCH 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2 > > > > From: Roy Pledge <Roy.Pledge@xxxxxxx> > > > > Add QBman APIs for frame queue and buffer pool operations. > > > > Signed-off-by: Roy Pledge <roy.pledge@xxxxxxx> > > Signed-off-by: Haiying Wang <haiying.wang@xxxxxxx> > > Signed-off-by: Stuart Yoder <stuart.yoder@xxxxxxx> > > --- > > drivers/bus/fsl-mc/dpio/Makefile | 2 +- > > drivers/bus/fsl-mc/dpio/qbman-portal.c | 1009 > > ++++++++++++++++++++++++++++++++ > > drivers/bus/fsl-mc/dpio/qbman-portal.h | 464 +++++++++++++++ > > 3 files changed, 1474 insertions(+), 1 deletion(-) > > create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.c > > create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.h > > > > diff --git a/drivers/bus/fsl-mc/dpio/Makefile b/drivers/bus/fsl- > > mc/dpio/Makefile > > index 128befc..6588498 100644 > > --- a/drivers/bus/fsl-mc/dpio/Makefile > > +++ b/drivers/bus/fsl-mc/dpio/Makefile > > @@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror > > > > obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o > > > > -fsl-mc-dpio-objs := dpio.o > > +fsl-mc-dpio-objs := dpio.o qbman-portal.o > > diff --git a/drivers/bus/fsl-mc/dpio/qbman-portal.c b/drivers/bus/fsl- > > mc/dpio/qbman-portal.c > > new file mode 100644 > > index 0000000..1eb3dd9 > > --- /dev/null > > +++ b/drivers/bus/fsl-mc/dpio/qbman-portal.c > > @@ -0,0 +1,1009 @@ > > [...] > > > +/** > > + * qbman_swp_release() - Issue a buffer release command > > + * @s: the software portal object > > + * @d: the release descriptor > > + * @buffers: a pointer pointing to the buffer address to be released > > + * @num_buffers: number of buffers to be released, must be less than 8 > > + * > > + * Return 0 for success, -EBUSY if the release command ring is not ready. > > + */ > > +int qbman_swp_release(struct qbman_swp *s, const struct > > qbman_release_desc *d, > > + const u64 *buffers, unsigned int num_buffers) > > +{ > > + int i; > > + struct qbman_release_desc *p; > > + u32 rar; > > + > > + if (!num_buffers || (num_buffers > 7)) > > + return -EINVAL; > > + > > + rar = qbman_read_register(s, QBMAN_CINH_SWP_RAR); > > + if (!RAR_SUCCESS(rar)) > > + return -EBUSY; > > + > > + /* Start the release command */ > > + p = qbman_get_cmd(s, QBMAN_CENA_SWP_RCR(RAR_IDX(rar))); > > + /* Copy the caller's buffer pointers to the command */ > > + for (i = 0; i < num_buffers; i++) > > + p->buf[i] = cpu_to_le64(buffers[i]); > > + > > Hi Stuart, > We also need to set BPID field in the buffer release command, something like: > + p->bpid = d->bpid; > Without this all buffers will be released to buffer pool id 0, which is incorrect. Will fix on next respin. > > + /* > > + * Set the verb byte, have to substitute in the valid-bit and the > > number > > + * of buffers. > > + */ > > + dma_wmb(); > > + p->verb = d->verb | RAR_VB(rar) | num_buffers; > > + > > + return 0; > > +} > > + > > +struct qbman_acquire_desc { > > + u8 verb; > > + u8 reserved; > > + u16 bpid; > > + u8 num; > > + u8 reserved2[59]; > > +}; > > + > > +struct qbman_acquire_rslt { > > + u8 verb; > > + u8 rslt; > > + u16 reserved; > > + u8 num; > > + u8 reserved2[3]; > > + u64 buf[7]; > > +}; > > + > > +/** > > + * qbman_swp_acquire() - Issue a buffer acquire command > > + * @s: the software portal object > > + * @bpid: the buffer pool index > > + * @buffers: a pointer pointing to the acquired buffer addresses > > + * @num_buffers: number of buffers to be acquired, must be less than 8 > > + * > > + * Return 0 for success, or negative error code if the acquire command > > + * fails. > > + */ > > +int qbman_swp_acquire(struct qbman_swp *s, u16 bpid, u64 *buffers, > > + unsigned int num_buffers) > > +{ > > + struct qbman_acquire_desc *p; > > + struct qbman_acquire_rslt *r; > > + int i; > > + > > + if (!num_buffers || (num_buffers > 7)) > > + return -EINVAL; > > + > > + /* Start the management command */ > > + p = qbman_swp_mc_start(s); > > qbman_swp_mc_start() returns a pointer to where the QBMan > management command must be written, but doesn't clear any > previous values found there. > We should memset the area to zero before using it. > Same comment applies to other places where this function is used. Ok, will fix. > > + > > + if (!p) > > + return -EBUSY; > > + > > + /* Encode the caller-provided attributes */ > > + p->bpid = cpu_to_le16(bpid); > > + p->num = num_buffers; > > + > > + /* Complete the management command */ > > + r = qbman_swp_mc_complete(s, p, p->verb | > > QBMAN_MC_ACQUIRE); > > qbman_swp_mc_complete() may return NULL, if for instance the hardware > is unresponsive. We need to check this before dereferencing r. > Same for other instances of usage throughout the code. Ok, will fix. Thanks for the review. Stuart _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel