Hi Catalin
> -----Original Message-----
> From: Catalin Marinas <[email protected]>
> Sent: 2019年9月24日 0:07
> To: Justin He (Arm Technology China) <[email protected]>
> Cc: Will Deacon <[email protected]>; Mark Rutland
> <[email protected]>; James Morse <[email protected]>; Marc
> Zyngier <[email protected]>; Matthew Wilcox <[email protected]>; Kirill A.
> Shutemov <[email protected]>; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; Suzuki Poulose <[email protected]>; Punit
> Agrawal <[email protected]>; Anshuman Khandual
> <[email protected]>; Alex Van Brunt
> <[email protected]>; Robin Murphy <[email protected]>;
> Thomas Gleixner <[email protected]>; Andrew Morton <akpm@linux-
> foundation.org>; Jérôme Glisse <[email protected]>; Ralph Campbell
> <[email protected]>; [email protected]; Kaly Xin (Arm Technology
> China) <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v8 1/3] arm64: cpufeature: introduce helper
> cpu_has_hw_af()
>
> On Sat, Sep 21, 2019 at 09:50:52PM +0800, Jia He wrote:
> > We unconditionally set the HW_AFDBM capability and only enable it on
> > CPUs which really have the feature. But sometimes we need to know
> > whether this cpu has the capability of HW AF. So decouple AF from
> > DBM by new helper cpu_has_hw_af().
> >
> > Reported-by: kbuild test robot <[email protected]>
> > Suggested-by: Suzuki Poulose <[email protected]>
> > Signed-off-by: Jia He <[email protected]>
> > ---
> > arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/cpufeature.h
> b/arch/arm64/include/asm/cpufeature.h
> > index c96ffa4722d3..46caf934ba4e 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -667,6 +667,16 @@ static inline u32
> id_aa64mmfr0_parange_to_phys_shift(int parange)
> > default: return CONFIG_ARM64_PA_BITS;
> > }
> > }
> > +
> > +/* Decouple AF from AFDBM. */
>
> We could do with a better comment here or just remove it altogether. The
> aim of the patch was to decouple AF check from the AF+DBM but the
> comment here should describe what the function does. Maybe something
> like: "Check whether hardware update of the Access flag is supported".
>
Okay, I will update it
--
Cheers,
Justin (Jia He)
> > +static inline bool cpu_has_hw_af(void)
> > +{
> > + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
> > + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf;
> > +
> > + return false;
> > +}
>
> Other than the comment above,
>
> Reviewed-by: Catalin Marinas <[email protected]>
