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

[James Smart <jsmart2021@gmail.com>]

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.