Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: refactor some missing field get/prep conversions

[Alexander Lobakin <aleksander.lobakin@intel.com>]

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
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan