lkml.org 
[lkml]   [2015]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 09/24] arm64: Keep track of CPU feature registers
From
Date
On 15/10/15 11:36, Catalin Marinas wrote:
> Hi Suzuki,
>
> Some minor comments below.

>> The feature bits are classified as one of SCALAR_MIN, SCALAR_MAX and DISCRETE
>> depending on the implication of the possible values. This information
>> is used to decide the safe value for a feature.
>>
>
> The "SCALAR..." etc. in the comment needs updating.

Sorry about that. Will fix it.

>
>> +#define FTR_STRICT true
>> +#define FTR_NONSTRICT false
>
> Please add a comment on what STRICT/NONSTRICT mean.
>

Sure

>> +static inline u64 ftr_mask(struct arm64_ftr_bits *ftrp)
>> +{
>> + return (u64)GENMASK(ftrp->shift + ftrp->width - 1, ftrp->shift);
>> +}
>> +
>> +static inline s64 arm64_ftr_value(struct arm64_ftr_bits *ftrp, u64 val)
>> +{
>> + return cpuid_feature_extract_field_width(val, ftrp->shift, ftrp->width);
>> +}
>
> Slightly inconsistent naming: since you are prefixing everything with
> arm64_, do the same for ftr_mask.

OK, will add it.

>
>> +static struct arm64_ftr_reg arm64_ftr_regs[] = {
>> +
>> + /* This array should be always kept in the ascending order of id */
>
> Do we have a sanity check somewhere? You could also call sort() on this
> array and we wouldn't have to worry.

No we don't have a sanity check. I will take a look.

>
>> +/*
>> + * get_arm64_sys_reg - Lookup a feature register entry using its
>> + * sys_reg() encoding. With the array arm64_ftr_regs sorted in the
>> + * ascending order, we use binary search to find a matching entry.
>> + *
>> + * returns - Upon success, matching ftr_reg entry for id.
>> + * - NULL on failure. It is upto the caller to decide
>> + * the impact of a failure.
>> + */
>> +static struct arm64_ftr_reg* get_arm64_sys_reg(u32 sys_id)
>
> Nitpick: the * near the function name.
>
> Also rename it to get_arm64_ftr_reg() to match the actual return type.
>

Sure, will fix all of these. Thanks for the detailed look





\
 
 \ /
  Last update: 2015-10-15 13:01    [W:0.089 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site