Re: [PATCH 01/32] elx: libefc_sli: SLI-4 register offsets and field definitions

James Smart
2019-10-25T23:04:25Z

Thanks. We mostly agree with the comment written and will work on the
changes.

Exceptions or answers to questions are inline below.

-- james

On 10/24/2019 9:22 AM, Daniel Wagner wrote:

>> + SLI4_INTF_VALID_SHIFT = 29,
>> + SLI4_INTF_VALID_MASK = 0x0F << SLI4_INTF_VALID_SHIFT,
>
> Should this a 32 bit value? This overflows to 34 bits.

agreed

>
>> +
>> + SLI4_INTF_VALID_VALUE = 6 << SLI4_INTF_VALID_SHIFT,
>> +};
>
> Just style question: what is the benefit using anonymous enums? The
> only reason I came up was that gdb could show the name of the
> value. Though a quick test didn't work if the value is passed into a
> function. Maybe I did something wrong.
>
> I am asking because register number is a define and then the shift and
> mask are enums.

In newer code I've seen the preference being anonymous enums. But in
looking for why or when to use what, there wasn't much guidance or
reasons. As in my other email, we had older defines. In newer code we
used enums. So we have a mix. At this point, there's so much volume it's
not worth making it one way or the other.

>> +/**
>> + * @brief MQ_DOORBELL - MQ Doorbell Register
>> + */
>> +#define SLI4_MQ_DB_REG 0x0140 /* register offset */
>
> Are the other registers defines also all offsets? Just wondering if
> the comment is pointing out that these values are special or not.

yes. We'll clarify them as well.