Subject: Re: [2.6.19 PATCH 5/7] ehea: main header files

From: Thomas Klein <[email-masked]>
Date: Mon, 21 Aug 2006 14:03:32 +0200

Michael Neuling wrote:
 >> +static inline void ehea_update_sqa(struct ehea_qp *qp, u16 nr_wqes)
 >> +{
 >> +    struct h_epa epa = qp->epas.kernel;
 >> +    epa_store_acc(epa, QPTEMM_OFFSET(qpx_sqa),
 >> +                  EHEA_BMASK_SET(QPX_SQA_VALUE, nr_wqes));
 >> +}
 >> +
 >> +static inline void ehea_update_rq3a(struct ehea_qp *qp, u16 nr_wqes)
 >> +{
 >> +    struct h_epa epa = qp->epas.kernel;
 >> +    epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq3a),
 >> +                  EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
 >> +}
 >> +
 >> +static inline void ehea_update_rq2a(struct ehea_qp *qp, u16 nr_wqes)
 >> +{
 >> +    struct h_epa epa = qp->epas.kernel;
 >> +    epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq2a),
 >> +                  EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
 >> +}
 >> +
 >> +static inline void ehea_update_rq1a(struct ehea_qp *qp, u16 nr_wqes)
 >> +{
 >> +    struct h_epa epa = qp->epas.kernel;
 >> +    epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq1a),
 >> +                  EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
 >> +}
 >> +
 >> +static inline void ehea_update_feca(struct ehea_cq *cq, u32 nr_cqes)
 >> +{
 >> +    struct h_epa epa = cq->epas.kernel;
 >> +    epa_store_acc(epa, CQTEMM_OFFSET(cqx_feca),
 >> +                  EHEA_BMASK_SET(CQX_FECADDER, nr_cqes));
 >> +}
 >> +
 >> +static inline void ehea_reset_cq_n1(struct ehea_cq *cq)
 >> +{
 >> +    struct h_epa epa = cq->epas.kernel;
 >> +    epa_store_cq(epa, cqx_n1,
 >> +                 EHEA_BMASK_SET(CQX_N1_GENERATE_COMP_EVENT, 1));
 >> +}
 >> +
 >> +static inline void ehea_reset_cq_ep(struct ehea_cq *my_cq)
 >> +{
 >> +    struct h_epa epa = my_cq->epas.kernel;
 >> +    epa_store_acc(epa, CQTEMM_OFFSET(cqx_ep),
 >> +                  EHEA_BMASK_SET(CQX_EP_EVENT_PENDING, 0));
 >> +}
 >
 > These are almost identical... I'm sure most (if not all) could be merged
 > into a single function or #define.
 >
 > Mikey

Hi Mikey,

I gave it a try: ehea_reset_cq_n1() drops out because it calls epa_store_cq(),
not epa_store_acc(). ehea_update_feca() and ehea_reset_cq_ep() require a
different input parm as the others and replacing two inline functions by
one inline function and two macros doesn't help neither the code nor does
it improve readability.
Finally we have ehea_update_sqa() and the 3 ehea_update_rqXa() functions which
I replaced by an inline function and four macros. See the result below. It
think understanding what this does is way more difficult than looking at the
four inline functions we had before. Therefore I'd prefer leaving those inline
functions as is.

Regards
Thomas

#define ehea_update_sqa(qp, nr_wqes) \
         ehea_update_qa(qp, nr_wqes, \
                        QPTEMM_OFFSET(qpx_sqa), \
                        EHEA_BMASK_SET(QPX_SQA_VALUE, nr_wqes));

#define ehea_update_rq1a(qp, nr_wqes) \
         ehea_update_qa(qp, nr_wqes, \
                        QPTEMM_OFFSET(qpx_rq1a), \
                        EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));

#define ehea_update_rq2a(qp, nr_wqes) \
         ehea_update_qa(qp, nr_wqes, \
                        QPTEMM_OFFSET(qpx_rq2a), \
                        EHEA_BMASK_SET(QPX_RQ2A_VALUE, nr_wqes));

#define ehea_update_rq3a(qp, nr_wqes) \
         ehea_update_qa(qp, nr_wqes, \
                        QPTEMM_OFFSET(qpx_rq3a), \
                        EHEA_BMASK_SET(QPX_RQ3A_VALUE, nr_wqes));

static inline void ehea_update_qa(struct ehea_qp *qp, u16 nr_wqes,
                                   u32 offset, u64 value)
{
         struct h_epa epa = qp->epas.kernel;
         epa_store_acc(epa, offset, value);
}