| Message ID | 20231130214511.647586-1-jesse.brandeburg@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-next,v1] idpf: refactor some missing field get/prep conversions | expand |
On 11/30/23 22:45, Jesse Brandeburg wrote: > Most of idpf correctly uses FIELD_GET and FIELD_PREP, but a couple spots > were missed so fix those. > > This conversion was automated via a coccinelle script as posted with the > previous series. [1] > > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > --- > This patch should be applied after the larger FIELD_PREP/FIELD_GET > conversion series for the Intel drivers. or at the end of, so [1] would be self-explanatory > --- > .../ethernet/intel/idpf/idpf_singleq_txrx.c | 7 +++---- > drivers/net/ethernet/intel/idpf/idpf_txrx.c | 18 ++++++++---------- > 2 files changed, 11 insertions(+), 14 deletions(-) > Anyway, it is better code so, Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com> Date: Thu, 30 Nov 2023 13:45:11 -0800 > Most of idpf correctly uses FIELD_GET and FIELD_PREP, but a couple spots > were missed so fix those. > > This conversion was automated via a coccinelle script as posted with the > previous series. > > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> > --- > This patch should be applied after the larger FIELD_PREP/FIELD_GET > conversion series for the Intel drivers. > --- > .../ethernet/intel/idpf/idpf_singleq_txrx.c | 7 +++---- > drivers/net/ethernet/intel/idpf/idpf_txrx.c | 18 ++++++++---------- > 2 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c > index 81288a17da2a..447753495c53 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c > @@ -328,10 +328,9 @@ static void idpf_tx_singleq_build_ctx_desc(struct idpf_queue *txq, > > if (offload->tso_segs) { > qw1 |= IDPF_TX_CTX_DESC_TSO << IDPF_TXD_CTX_QW1_CMD_S; > - qw1 |= ((u64)offload->tso_len << IDPF_TXD_CTX_QW1_TSO_LEN_S) & > - IDPF_TXD_CTX_QW1_TSO_LEN_M; > - qw1 |= ((u64)offload->mss << IDPF_TXD_CTX_QW1_MSS_S) & > - IDPF_TXD_CTX_QW1_MSS_M; > + qw1 |= FIELD_PREP(IDPF_TXD_CTX_QW1_TSO_LEN_M, > + offload->tso_len); > + qw1 |= FIELD_PREP(IDPF_TXD_CTX_QW1_MSS_M, offload->mss); > > u64_stats_update_begin(&txq->stats_sync); > u64_stats_inc(&txq->q_stats.tx.lso_pkts); > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c > index 1f728a9004d9..f3009d2a3c2e 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c > @@ -505,7 +505,7 @@ static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id) > > /* store the buffer ID and the SW maintained GEN bit to the refillq */ > refillq->ring[nta] = > - ((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) | > + FIELD_PREP(IDPF_RX_BI_BUFID_M, buf_id) | > (!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) << > IDPF_RX_BI_GEN_S); Why isn't that one converted as well? > > @@ -1825,14 +1825,14 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, > u16 gen; > > /* if the descriptor isn't done, no work yet to do */ > - gen = (le16_to_cpu(tx_desc->qid_comptype_gen) & > - IDPF_TXD_COMPLQ_GEN_M) >> IDPF_TXD_COMPLQ_GEN_S; > + gen = FIELD_GET(IDPF_TXD_COMPLQ_GEN_M, > + le16_to_cpu(tx_desc->qid_comptype_gen)); The definition: #define IDPF_TXD_COMPLQ_GEN_M BIT_ULL(IDPF_TXD_COMPLQ_GEN_S) Please don't use FIELD_*() API for 1 bit. gen = !!(le16_to_cpu(tx_desc->qid_comptype_gen) & IDPF_TXD_COMPLQ_GEN_M); is enough. Moreover, you could use le*_{get,encode,replace}_bits() to get/set LE values right away without 2-step operation (from/to CPU + masks), but you didn't do that here (see below for an example). > if (test_bit(__IDPF_Q_GEN_CHK, complq->flags) != gen) > break; > > /* Find necessary info of TX queue to clean buffers */ > - rel_tx_qid = (le16_to_cpu(tx_desc->qid_comptype_gen) & > - IDPF_TXD_COMPLQ_QID_M) >> IDPF_TXD_COMPLQ_QID_S; > + rel_tx_qid = FIELD_GET(IDPF_TXD_COMPLQ_QID_M, > + le16_to_cpu(tx_desc->qid_comptype_gen)); rel_tx_qid = le16_get_bits(tx_desc->qid_comptype_gen, IDPF_TXD_COMPLQ_QID_M); > if (rel_tx_qid >= complq->txq_grp->num_txq || > !complq->txq_grp->txqs[rel_tx_qid]) { > dev_err(&complq->vport->adapter->pdev->dev, > @@ -1842,9 +1842,8 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, > tx_q = complq->txq_grp->txqs[rel_tx_qid]; > > /* Determine completion type */ > - ctype = (le16_to_cpu(tx_desc->qid_comptype_gen) & > - IDPF_TXD_COMPLQ_COMPL_TYPE_M) >> > - IDPF_TXD_COMPLQ_COMPL_TYPE_S; > + ctype = FIELD_GET(IDPF_TXD_COMPLQ_COMPL_TYPE_M, > + le16_to_cpu(tx_desc->qid_comptype_gen)); Same. > switch (ctype) { > case IDPF_TXD_COMPLT_RE: > hw_head = le16_to_cpu(tx_desc->q_head_compl_tag.q_head); > @@ -1947,8 +1946,7 @@ void idpf_tx_splitq_build_ctb(union idpf_tx_flex_desc *desc, > desc->q.qw1.cmd_dtype = > cpu_to_le16(params->dtype & IDPF_FLEX_TXD_QW1_DTYPE_M); > desc->q.qw1.cmd_dtype |= > - cpu_to_le16((td_cmd << IDPF_FLEX_TXD_QW1_CMD_S) & > - IDPF_FLEX_TXD_QW1_CMD_M); > + cpu_to_le16(FIELD_PREP(IDPF_FLEX_TXD_QW1_CMD_M, td_cmd)); Same. > desc->q.qw1.buf_size = cpu_to_le16((u16)size); > desc->q.qw1.l2tags.l2tag1 = cpu_to_le16(params->td_tag); > } In general, I would say those two issues are very common in IDPF and also the whole your series converting the Intel drivers. The scripts won't check whether the mask has only 1 bit or whether the value gets converted from/to LE, so they won't help here. Could you maybe manually recheck all the places where bitfield masks are used at least in IDPF (better in ice, iavf, i40e, ..., as well) and posted a series that would address them? At the end, manual work is more valuable than automated conversions :p Thanks, Olek
On 12/1/2023 6:32 AM, Alexander Lobakin wrote: > From: Jesse Brandeburg <jesse.brandeburg@intel.com> >> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c >> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c >> @@ -505,7 +505,7 @@ static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id) >> >> /* store the buffer ID and the SW maintained GEN bit to the refillq */ >> refillq->ring[nta] = >> - ((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) | >> + FIELD_PREP(IDPF_RX_BI_BUFID_M, buf_id) | >> (!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) << >> IDPF_RX_BI_GEN_S); > > Why isn't that one converted as well? Because it's not a constant, and it's not checking a mask with "&", so the automation ignored it. I *did* a test, and we could convert the return value from test_bit (a bool) into the IDPF_RX_BI_GEN_M mask with FIELD_PREP, since C-code allows the luxury of converting a bool to a "1", even though it's a bit type ugly in this age of strict typing. > >> >> @@ -1825,14 +1825,14 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, >> u16 gen; >> >> /* if the descriptor isn't done, no work yet to do */ >> - gen = (le16_to_cpu(tx_desc->qid_comptype_gen) & >> - IDPF_TXD_COMPLQ_GEN_M) >> IDPF_TXD_COMPLQ_GEN_S; >> + gen = FIELD_GET(IDPF_TXD_COMPLQ_GEN_M, >> + le16_to_cpu(tx_desc->qid_comptype_gen)); > > The definition: > > #define IDPF_TXD_COMPLQ_GEN_M BIT_ULL(IDPF_TXD_COMPLQ_GEN_S) > > Please don't use FIELD_*() API for 1 bit. Did you mean that gen is effectively used as a bool? I think that has nothing to do with my change over to FIELD_GET, but I could see how redesigning this code would be useful, but not as part of this conversion series. > > gen = !!(le16_to_cpu(tx_desc->qid_comptype_gen) & > IDPF_TXD_COMPLQ_GEN_M); > > is enough. Generally I'd prefer that the kind of check above returned a bool with a constant conversion of the mask (compile time) to an LE16 mask, and then use that, which is why all of our other drivers do that instead. > > Moreover, you could use le*_{get,encode,replace}_bits() to get/set LE > values right away without 2-step operation (from/to CPU + masks), but > you didn't do that here (see below for an example). Those aren't widely used yet in our drivers so I wasn't picking them up yet. But thank you for pointing that out. <snip> > In general, I would say those two issues are very common in IDPF and > also the whole your series converting the Intel drivers. The scripts > won't check whether the mask has only 1 bit or whether the value gets > converted from/to LE, so they won't help here. I had been hoping to do some more followup work. it's possible that with some tweaking the coccinelle script could learn how to detect non-pow2 constants, and therefore possibly one bit constants as well. Maybe @Julia can help us refine the script and possibly get it into the scripts/coccinelle directory to help other drivers as well. > Could you maybe manually recheck all the places where bitfield masks are > used at least in IDPF (better in ice, iavf, i40e, ..., as well) and > posted a series that would address them? At the end, manual work is more > valuable than automated conversions :p I think a followup series would work better for this, do you agree? Thanks, Jesse
On Fri, 1 Dec 2023, Jesse Brandeburg wrote: > On 12/1/2023 6:32 AM, Alexander Lobakin wrote: > > From: Jesse Brandeburg <jesse.brandeburg@intel.com> > > >> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c > >> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c > >> @@ -505,7 +505,7 @@ static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id) > >> > >> /* store the buffer ID and the SW maintained GEN bit to the refillq */ > >> refillq->ring[nta] = > >> - ((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) | > >> + FIELD_PREP(IDPF_RX_BI_BUFID_M, buf_id) | > >> (!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) << > >> IDPF_RX_BI_GEN_S); > > > > Why isn't that one converted as well? > > Because it's not a constant, and it's not checking a mask with "&", so > the automation ignored it. I *did* a test, and we could convert the > return value from test_bit (a bool) into the IDPF_RX_BI_GEN_M mask with > FIELD_PREP, since C-code allows the luxury of converting a bool to a > "1", even though it's a bit type ugly in this age of strict typing. > > > > >> > >> @@ -1825,14 +1825,14 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, > >> u16 gen; > >> > >> /* if the descriptor isn't done, no work yet to do */ > >> - gen = (le16_to_cpu(tx_desc->qid_comptype_gen) & > >> - IDPF_TXD_COMPLQ_GEN_M) >> IDPF_TXD_COMPLQ_GEN_S; > >> + gen = FIELD_GET(IDPF_TXD_COMPLQ_GEN_M, > >> + le16_to_cpu(tx_desc->qid_comptype_gen)); > > > > The definition: > > > > #define IDPF_TXD_COMPLQ_GEN_M BIT_ULL(IDPF_TXD_COMPLQ_GEN_S) > > > > Please don't use FIELD_*() API for 1 bit. > > Did you mean that gen is effectively used as a bool? I think that has > nothing to do with my change over to FIELD_GET, but I could see how > redesigning this code would be useful, but not as part of this > conversion series. > > > > > gen = !!(le16_to_cpu(tx_desc->qid_comptype_gen) & > > IDPF_TXD_COMPLQ_GEN_M); > > > > is enough. > > Generally I'd prefer that the kind of check above returned a bool with a > constant conversion of the mask (compile time) to an LE16 mask, and then > use that, which is why all of our other drivers do that instead. > > > > > Moreover, you could use le*_{get,encode,replace}_bits() to get/set LE > > values right away without 2-step operation (from/to CPU + masks), but > > you didn't do that here (see below for an example). > > Those aren't widely used yet in our drivers so I wasn't picking them up > yet. But thank you for pointing that out. > > <snip> > > > > In general, I would say those two issues are very common in IDPF and > > also the whole your series converting the Intel drivers. The scripts > > won't check whether the mask has only 1 bit or whether the value gets > > converted from/to LE, so they won't help here. > > I had been hoping to do some more followup work. it's possible that with > some tweaking the coccinelle script could learn how to detect non-pow2 > constants, and therefore possibly one bit constants as well. Maybe > @Julia can help us refine the script and possibly get it into the > scripts/coccinelle directory to help other drivers as well. I don't have the original script handy. If it is an explicit constant, like 8, then you can match it with something like: constant pow2 : script:python() { is_pow2(pow2) }; where is_pow2 is a python function that first converts pow2 to an integer, and then, if that succeeds, checks if it is a power of two. The result of is_pow2 should be true or false. When there is a #define constant, then you can to match the #define to determine the value. This may require options like --all-includes or --recursive-includes. Then you can write: @r@ constant pow2 : script:python() { is_pow2(pow2) }; identifier i; @@ #define i pow2 and then in some later rule, you can have identifier r.i; and then use i in a place where you expect a power of two constant. julia > > > Could you maybe manually recheck all the places where bitfield masks are > > used at least in IDPF (better in ice, iavf, i40e, ..., as well) and > > posted a series that would address them? At the end, manual work is more > > valuable than automated conversions :p > > I think a followup series would work better for this, do you agree? > > Thanks, > Jesse >
From: Jesse Brandeburg <jesse.brandeburg@intel.com> Date: Fri, 1 Dec 2023 12:12:05 -0800 > On 12/1/2023 6:32 AM, Alexander Lobakin wrote: >> From: Jesse Brandeburg <jesse.brandeburg@intel.com> > >>> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c >>> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c >>> @@ -505,7 +505,7 @@ static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id) >>> >>> /* store the buffer ID and the SW maintained GEN bit to the refillq */ >>> refillq->ring[nta] = >>> - ((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) | >>> + FIELD_PREP(IDPF_RX_BI_BUFID_M, buf_id) | >>> (!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) << >>> IDPF_RX_BI_GEN_S); >> >> Why isn't that one converted as well? > > Because it's not a constant, and it's not checking a mask with "&", so > the automation ignored it. I *did* a test, and we could convert the > return value from test_bit (a bool) into the IDPF_RX_BI_GEN_M mask with > FIELD_PREP, since C-code allows the luxury of converting a bool to a > "1", even though it's a bit type ugly in this age of strict typing. What is "not a constant"? ring[nta] = FIELD_PREP(IDPF_RX_BI_GEN_M, test_bit(__IDPF_Q_GEN_CHK, flags)); Is there a problem with this ^ code? "The scripts ignored that" is not a good argument I'd say :> > >> >>> >>> @@ -1825,14 +1825,14 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, >>> u16 gen; >>> >>> /* if the descriptor isn't done, no work yet to do */ >>> - gen = (le16_to_cpu(tx_desc->qid_comptype_gen) & >>> - IDPF_TXD_COMPLQ_GEN_M) >> IDPF_TXD_COMPLQ_GEN_S; >>> + gen = FIELD_GET(IDPF_TXD_COMPLQ_GEN_M, >>> + le16_to_cpu(tx_desc->qid_comptype_gen)); >> >> The definition: >> >> #define IDPF_TXD_COMPLQ_GEN_M BIT_ULL(IDPF_TXD_COMPLQ_GEN_S) >> >> Please don't use FIELD_*() API for 1 bit. > > Did you mean that gen is effectively used as a bool? I think that has > nothing to do with my change over to FIELD_GET, but I could see how > redesigning this code would be useful, but not as part of this > conversion series. > >> >> gen = !!(le16_to_cpu(tx_desc->qid_comptype_gen) & >> IDPF_TXD_COMPLQ_GEN_M); >> >> is enough. > > Generally I'd prefer that the kind of check above returned a bool with a > constant conversion of the mask (compile time) to an LE16 mask, and then > use that, which is why all of our other drivers do that instead. Ah, good point. Smth like gen = !!(tx_desc->qid_comptype_gen & IDPF_TXQ_COMPLQ_GEN_M_LE); OTOH x86 is always LE and BE is seen more and more rarely nowadays. It might just not worth having a LE-version of each such mask for the sake of a bit more optimized code on architectures where our drivers are barely used. > >> >> Moreover, you could use le*_{get,encode,replace}_bits() to get/set LE >> values right away without 2-step operation (from/to CPU + masks), but >> you didn't do that here (see below for an example). > > Those aren't widely used yet in our drivers so I wasn't picking them up > yet. But thank you for pointing that out. > > <snip> > > >> In general, I would say those two issues are very common in IDPF and >> also the whole your series converting the Intel drivers. The scripts >> won't check whether the mask has only 1 bit or whether the value gets >> converted from/to LE, so they won't help here. > > I had been hoping to do some more followup work. it's possible that with > some tweaking the coccinelle script could learn how to detect non-pow2 > constants, and therefore possibly one bit constants as well. Maybe > @Julia can help us refine the script and possibly get it into the > scripts/coccinelle directory to help other drivers as well. Every automated change needs polishing by human. Don't FIELD_*() macros already check whether the passed mask is actually a contiguous mask? > >> Could you maybe manually recheck all the places where bitfield masks are >> used at least in IDPF (better in ice, iavf, i40e, ..., as well) and >> posted a series that would address them? At the end, manual work is more >> valuable than automated conversions :p > > I think a followup series would work better for this, do you agree? Nope :D If you want to convert all of our drivers to use bitfield API, I'd like to see a complete mature series instead of incremental series of series where followups will refactor the code introduced in the earlier ones. > > Thanks, > Jesse Thanks, Olek
On 12/4/2023 2:26 AM, Alexander Lobakin wrote: yping. > > What is "not a constant"? > > ring[nta] = FIELD_PREP(IDPF_RX_BI_GEN_M, > test_bit(__IDPF_Q_GEN_CHK, flags)); > > Is there a problem with this ^ code? > > "The scripts ignored that" is not a good argument I'd say :> Fixed in v2 >> Generally I'd prefer that the kind of check above returned a bool with a >> constant conversion of the mask (compile time) to an LE16 mask, and then >> use that, which is why all of our other drivers do that instead. > > Ah, good point. Smth like > > gen = !!(tx_desc->qid_comptype_gen & > IDPF_TXQ_COMPLQ_GEN_M_LE); Yeah, it would be nice but I didn't add that to this series. But you have the idea. > > OTOH x86 is always LE and BE is seen more and more rarely nowadays. It > might just not worth having a LE-version of each such mask for the sake > of a bit more optimized code on architectures where our drivers are > barely used. Our drivers should work on BE, IMO. sparse annotations takes care of making sure we have the conversions right, they cost nothing on x86. > >> >>> >>> Moreover, you could use le*_{get,encode,replace}_bits() to get/set LE >>> values right away without 2-step operation (from/to CPU + masks), but >>> you didn't do that here (see below for an example). Done in v2. >> >> Those aren't widely used yet in our drivers so I wasn't picking them up >> yet. But thank you for pointing that out. >> >> <snip> >> >> >>> In general, I would say those two issues are very common in IDPF and >>> also the whole your series converting the Intel drivers. The scripts >>> won't check whether the mask has only 1 bit or whether the value gets >>> converted from/to LE, so they won't help here. >> >> I had been hoping to do some more followup work. it's possible that with >> some tweaking the coccinelle script could learn how to detect non-pow2 >> constants, and therefore possibly one bit constants as well. Maybe >> @Julia can help us refine the script and possibly get it into the >> scripts/coccinelle directory to help other drivers as well. > > Every automated change needs polishing by human. > > Don't FIELD_*() macros already check whether the passed mask is actually > a contiguous mask? Yes > >> >>> Could you maybe manually recheck all the places where bitfield masks are >>> used at least in IDPF (better in ice, iavf, i40e, ..., as well) and >>> posted a series that would address them? At the end, manual work is more >>> valuable than automated conversions :p >> >> I think a followup series would work better for this, do you agree? > > Nope :D If you want to convert all of our drivers to use bitfield API, > I'd like to see a complete mature series instead of incremental series > of series where followups will refactor the code introduced in the > earlier ones. v2 of the previous series sent with audit of all *cpu* conversions and fixing up leXX_*_bits() opportunities. Appreciate the reviews!
From: Jesse Brandeburg <jesse.brandeburg@intel.com> Date: Tue, 5 Dec 2023 17:10:58 -0800 > On 12/4/2023 2:26 AM, Alexander Lobakin wrote: > yping. >> >> What is "not a constant"? >> >> ring[nta] = FIELD_PREP(IDPF_RX_BI_GEN_M, >> test_bit(__IDPF_Q_GEN_CHK, flags)); >> >> Is there a problem with this ^ code? >> >> "The scripts ignored that" is not a good argument I'd say :> > > Fixed in v2 > > >>> Generally I'd prefer that the kind of check above returned a bool with a >>> constant conversion of the mask (compile time) to an LE16 mask, and then >>> use that, which is why all of our other drivers do that instead. >> >> Ah, good point. Smth like >> >> gen = !!(tx_desc->qid_comptype_gen & >> IDPF_TXQ_COMPLQ_GEN_M_LE); > > Yeah, it would be nice but I didn't add that to this series. But you > have the idea. > >> >> OTOH x86 is always LE and BE is seen more and more rarely nowadays. It >> might just not worth having a LE-version of each such mask for the sake >> of a bit more optimized code on architectures where our drivers are >> barely used. > > Our drivers should work on BE, IMO. sparse annotations takes care of > making sure we have the conversions right, they cost nothing on x86. They do work, what I meant is that adding _LE constant masks would make the code a bit more optimized on BE only, then would it make sense to add +1 line per each such mask to get some almost invisible optimizations on non-common architectures? Thanks, Olek
diff --git a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c index 81288a17da2a..447753495c53 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c @@ -328,10 +328,9 @@ static void idpf_tx_singleq_build_ctx_desc(struct idpf_queue *txq, if (offload->tso_segs) { qw1 |= IDPF_TX_CTX_DESC_TSO << IDPF_TXD_CTX_QW1_CMD_S; - qw1 |= ((u64)offload->tso_len << IDPF_TXD_CTX_QW1_TSO_LEN_S) & - IDPF_TXD_CTX_QW1_TSO_LEN_M; - qw1 |= ((u64)offload->mss << IDPF_TXD_CTX_QW1_MSS_S) & - IDPF_TXD_CTX_QW1_MSS_M; + qw1 |= FIELD_PREP(IDPF_TXD_CTX_QW1_TSO_LEN_M, + offload->tso_len); + qw1 |= FIELD_PREP(IDPF_TXD_CTX_QW1_MSS_M, offload->mss); u64_stats_update_begin(&txq->stats_sync); u64_stats_inc(&txq->q_stats.tx.lso_pkts); diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c index 1f728a9004d9..f3009d2a3c2e 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c @@ -505,7 +505,7 @@ static void idpf_rx_post_buf_refill(struct idpf_sw_queue *refillq, u16 buf_id) /* store the buffer ID and the SW maintained GEN bit to the refillq */ refillq->ring[nta] = - ((buf_id << IDPF_RX_BI_BUFID_S) & IDPF_RX_BI_BUFID_M) | + FIELD_PREP(IDPF_RX_BI_BUFID_M, buf_id) | (!!(test_bit(__IDPF_Q_GEN_CHK, refillq->flags)) << IDPF_RX_BI_GEN_S); @@ -1825,14 +1825,14 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, u16 gen; /* if the descriptor isn't done, no work yet to do */ - gen = (le16_to_cpu(tx_desc->qid_comptype_gen) & - IDPF_TXD_COMPLQ_GEN_M) >> IDPF_TXD_COMPLQ_GEN_S; + gen = FIELD_GET(IDPF_TXD_COMPLQ_GEN_M, + le16_to_cpu(tx_desc->qid_comptype_gen)); if (test_bit(__IDPF_Q_GEN_CHK, complq->flags) != gen) break; /* Find necessary info of TX queue to clean buffers */ - rel_tx_qid = (le16_to_cpu(tx_desc->qid_comptype_gen) & - IDPF_TXD_COMPLQ_QID_M) >> IDPF_TXD_COMPLQ_QID_S; + rel_tx_qid = FIELD_GET(IDPF_TXD_COMPLQ_QID_M, + le16_to_cpu(tx_desc->qid_comptype_gen)); if (rel_tx_qid >= complq->txq_grp->num_txq || !complq->txq_grp->txqs[rel_tx_qid]) { dev_err(&complq->vport->adapter->pdev->dev, @@ -1842,9 +1842,8 @@ static bool idpf_tx_clean_complq(struct idpf_queue *complq, int budget, tx_q = complq->txq_grp->txqs[rel_tx_qid]; /* Determine completion type */ - ctype = (le16_to_cpu(tx_desc->qid_comptype_gen) & - IDPF_TXD_COMPLQ_COMPL_TYPE_M) >> - IDPF_TXD_COMPLQ_COMPL_TYPE_S; + ctype = FIELD_GET(IDPF_TXD_COMPLQ_COMPL_TYPE_M, + le16_to_cpu(tx_desc->qid_comptype_gen)); switch (ctype) { case IDPF_TXD_COMPLT_RE: hw_head = le16_to_cpu(tx_desc->q_head_compl_tag.q_head); @@ -1947,8 +1946,7 @@ void idpf_tx_splitq_build_ctb(union idpf_tx_flex_desc *desc, desc->q.qw1.cmd_dtype = cpu_to_le16(params->dtype & IDPF_FLEX_TXD_QW1_DTYPE_M); desc->q.qw1.cmd_dtype |= - cpu_to_le16((td_cmd << IDPF_FLEX_TXD_QW1_CMD_S) & - IDPF_FLEX_TXD_QW1_CMD_M); + cpu_to_le16(FIELD_PREP(IDPF_FLEX_TXD_QW1_CMD_M, td_cmd)); desc->q.qw1.buf_size = cpu_to_le16((u16)size); desc->q.qw1.l2tags.l2tag1 = cpu_to_le16(params->td_tag); }
Most of idpf correctly uses FIELD_GET and FIELD_PREP, but a couple spots were missed so fix those. This conversion was automated via a coccinelle script as posted with the previous series. Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com> --- This patch should be applied after the larger FIELD_PREP/FIELD_GET conversion series for the Intel drivers. --- .../ethernet/intel/idpf/idpf_singleq_txrx.c | 7 +++---- drivers/net/ethernet/intel/idpf/idpf_txrx.c | 18 ++++++++---------- 2 files changed, 11 insertions(+), 14 deletions(-)