Re: [RFC 4/7] soc/qman: add helper functions needed by caam/qi driver

[Scott Wood]

On Fri, 2017-03-03 at 16:52 +0200, Horia Geantă wrote:
> Add helper functions, macros, #defines for accessing / enabling
> qman functionality from caam/qi driver, such that this driver
> is not aware of the need for data conversion to big endian.

Why?  I complained about that (probably internally) when this driver was
originally submitted.

Having a bunch of accessors in a header file that just do an assignment with
an endian conversion just obfuscates things.  The driver still needs to know
that the conversion needs to happen, or else it wouldn't know that the fields
can't be accessed directly... and it gets particularly ridiculous when a
single field has a growing pile of accessors depending on exactly what flags
you want to set (e.g. qm_sg_entry_set_*).  Just open-code it unless an
accessor is justified by the call sites getting unwieldy or numerous.

It looks like GCC 6 has added an attribute (scalar_storage_order) that could
be used on structs to avoid having to manually swap such things.  I look
forward to the day when GCC 6 is old enough for the kernel to depend on this.

-Scott

[RFC 4/7] soc/qman: add helper functions needed by caam/qi driver

[Horia Geantă]

Add helper functions, macros, #defines for accessing / enabling
qman functionality from caam/qi driver, such that this driver
is not aware of the need for data conversion to big endian.
qman is updated to use these helpers internally.

Signed-off-by: Horia Geantă 
---
 drivers/soc/fsl/qbman/qman.c| 16 +--
 drivers/soc/fsl/qbman/qman_test_stash.c |  5 ++--
 include/soc/fsl/qman.h  | 47 +
 3 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 3d891db57ee6..7668ff53cd90 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1764,16 +1764,15 @@ int qman_init_fq(struct qman_fq *fq, u32 flags, struct 
qm_mcc_initfq *opts)
if (fq_isclear(fq, QMAN_FQ_FLAG_TO_DCPORTAL)) {
dma_addr_t phys_fq;
 
-   mcc->initfq.we_mask |= cpu_to_be16(QM_INITFQ_WE_CONTEXTB);
-   mcc->initfq.fqd.context_b = cpu_to_be32(fq_to_tag(fq));
+   qm_initfq_setbits(>initfq, QM_INITFQ_WE_CONTEXTB);
+   qm_fqd_set_contextb(>initfq.fqd, fq_to_tag(fq));
/*
 *  and the physical address - NB, if the user wasn't trying to
 * set CONTEXTA, clear the stashing settings.
 */
if (!(be16_to_cpu(mcc->initfq.we_mask) &
  QM_INITFQ_WE_CONTEXTA)) {
-   mcc->initfq.we_mask |=
-   cpu_to_be16(QM_INITFQ_WE_CONTEXTA);
+   qm_initfq_setbits(>initfq, QM_INITFQ_WE_CONTEXTA);
memset(>initfq.fqd.context_a, 0,
sizeof(mcc->initfq.fqd.context_a));
} else {
@@ -1795,8 +1794,7 @@ int qman_init_fq(struct qman_fq *fq, u32 flags, struct 
qm_mcc_initfq *opts)
 
if (!(be16_to_cpu(mcc->initfq.we_mask) &
  QM_INITFQ_WE_DESTWQ)) {
-   mcc->initfq.we_mask |=
-   cpu_to_be16(QM_INITFQ_WE_DESTWQ);
+   qm_initfq_setbits(>initfq, QM_INITFQ_WE_DESTWQ);
wq = 4;
}
qm_fqd_set_destwq(>initfq.fqd, p->config->channel, wq);
@@ -1816,7 +1814,7 @@ int qman_init_fq(struct qman_fq *fq, u32 flags, struct 
qm_mcc_initfq *opts)
}
if (opts) {
if (be16_to_cpu(opts->we_mask) & QM_INITFQ_WE_FQCTRL) {
-   if (be16_to_cpu(opts->fqd.fq_ctrl) & QM_FQCTRL_CGE)
+   if (qm_fqd_isset_fqctrl(>fqd, QM_FQCTRL_CGE))
fq_set(fq, QMAN_FQ_STATE_CGR_EN);
else
fq_clear(fq, QMAN_FQ_STATE_CGR_EN);
@@ -2321,7 +2319,7 @@ int qman_create_cgr(struct qman_cgr *cgr, u32 flags,
 
qm_cgr_cscn_targ_set(_opts.cgr, PORTAL_IDX(p),
 be32_to_cpu(cgr_state.cgr.cscn_targ));
-   local_opts.we_mask |= cpu_to_be16(QM_CGR_WE_CSCN_TARG);
+   qm_initcgr_setbits(_opts, QM_CGR_WE_CSCN_TARG);
 
/* send init if flags indicate so */
if (flags & QMAN_CGR_FLAG_USE_INIT)
@@ -2840,7 +2838,7 @@ static int cgr_cleanup(u32 cgrid)
err = qman_query_fq(, );
if (WARN_ON(err))
return err;
-   if (be16_to_cpu(fqd.fq_ctrl) & QM_FQCTRL_CGE &&
+   if (qm_fqd_isset_fqctrl(, QM_FQCTRL_CGE) &&
fqd.cgid == cgrid) {
pr_err("CRGID 0x%x is being used by FQID 0x%x, 
CGR will be leaked\n",
   cgrid, fq.fqid);
diff --git a/drivers/soc/fsl/qbman/qman_test_stash.c 
b/drivers/soc/fsl/qbman/qman_test_stash.c
index e87b65403b67..d2bf453092d7 100644
--- a/drivers/soc/fsl/qbman/qman_test_stash.c
+++ b/drivers/soc/fsl/qbman/qman_test_stash.c
@@ -406,9 +406,8 @@ static int init_handler(void *h)
goto failed;
}
memset(, 0, sizeof(opts));
-   opts.we_mask = cpu_to_be16(QM_INITFQ_WE_FQCTRL |
-  QM_INITFQ_WE_CONTEXTA);
-   opts.fqd.fq_ctrl = cpu_to_be16(QM_FQCTRL_CTXASTASHING);
+   qm_initfq_setbits(, QM_INITFQ_WE_FQCTRL | QM_INITFQ_WE_CONTEXTA);
+   qm_fqd_set_fqctrl(, QM_FQCTRL_CTXASTASHING);
qm_fqd_set_stashing(, 0, STASH_DATA_CL, STASH_CTX_CL);
err = qman_init_fq(>rx, QMAN_INITFQ_FLAG_SCHED |
   QMAN_INITFQ_FLAG_LOCAL, );
diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
index 0252c32f7421..fc133c658385 100644
--- a/include/soc/fsl/qman.h
+++ b/include/soc/fsl/qman.h
@@ -168,6 +168,12 @@ static inline void qm_fd_set_param(struct qm_fd *fd, enum 
qm_fd_format fmt,
 #define qm_fd_set_contig_big(fd, len) \