On 15.01.21 15:50, Marc Kleine-Budde wrote:
On 1/15/21 3:18 PM, Vincent MAILHOL wrote:
On Fri. 15 janv. 2021 at 22:51, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
On 1/15/21 2:20 PM, Vincent MAILHOL wrote:
/* map the data length to an appropriate data length code */
u8 can_fd_len2dlc(u8 len)
{
switch(len) {
case 0 ... 8:
return len;
case 9 ... 12:
return 9;
case 13 ... 16:
return 10;
case 17 ... 20:
return 11;
case 21 ... 24:
return 12;
case 25 ... 32:
return 13;
case 33 ... 48:
return 14;
case 49 ... 64:
default:
return CANFD_MAX_DLC;
}
}
And we will just leave the optimizations in the hand of the compiler.
I've already tried this. It results in a longer object file on ARM, even if you
remove the array....
You are right. I just checked the assembly code: it does a
dichotomy which means that in addition to being bigger, it is
also slower.
Please forget my previous message, it wasn't really smart.
I had the same idea, as I've never used these new cases I tried it out and was
underwhelmed. At least compared to this tightly packed array.
Yes. And for that reason I would like this patch to be completely reverted!
I'm fine with an improvement that adds some constant replacement like:
64 -> CANFD_MAX_LEN
0xF -> CANFD_MAX_DLC
But using ARRAY_SIZE() is not appropriate for a length conversion map.
And if you want to save 16 bytes and move the sanity check to a
functional check there has to be much more documentation in the code.
I was always happy having a generic 65 byte translation table which is
*extremely* fast in operation and which can be used *independently* from
can_fd_len2dlc().
There is no real need to replace that code with some error prone and
less understandable code.
Regards,
Oliver
