Re: [PATCH 03/32] elx: libefc_sli: Data structures and defines for mbox commands

James Smart
2019-10-25T22:42:19Z

Thanks. We mostly agree with the comment written and will work on the
changes. Agreed that consistency and uniformity helps.

Exceptions or answers to questions are inline below.

-- james

On 10/25/2019 4:19 AM, Daniel Wagner wrote:

>> +#define SLI_PAGE_SIZE (1 << 12) /* 4096 */
>
> So SLI_PAGE_SIZE is fixed and can't be changed...

For how the driver uses the SLI interface in this current
implementation, yes. The interface's page size is independent from what
the OS's pagesize is.

> ... and callers of this function pass in SLI_PAGE_SIZE.

>> +{
>> + u32 mask = page_size - 1;
>> + u32 shift = 0;
>> +
>> + switch (page_size) {
>> + case 4096:
>> + shift = 12;
>> + break;
>> + case 8192:
>> + shift = 13;
>> + break;
>> + case 16384:
>> + shift = 14;
>> + break;
>> + case 32768:
>> + shift = 15;
>> + break;
>> + case 65536:
>> + shift = 16;
>> + break;
>> + default:
>> + return 0;
>> + }
>
> What about using __ffs(page_size)? But...

will look into it...

>
>> +
>> + return (bytes + mask) >> shift;
>
> ... mask and shift could just be defined like SLI_PAGE_SIZE and we
> safe a few instructions. Unless SLI_PAGE_SIZE will be dynamic in future.

desire is to keep it easily adapted. There's actually some conditions
that could have us use different page sizes for different structures.