lkml.org 
[lkml]   [2023]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v4 3/5] iommu/arm-smmu: add ACTLR data and support for SM8550
    From


    On 12/16/2023 5:33 AM, Konrad Dybcio wrote:
    > On 15.12.2023 13:54, Robin Murphy wrote:
    >> On 2023-12-15 12:20 pm, Bibek Kumar Patro wrote:
    >>>
    >>>
    >>> On 12/15/2023 4:14 PM, Dmitry Baryshkov wrote:
    >>>> On Fri, 15 Dec 2023 at 12:19, Bibek Kumar Patro
    >>>> <quic_bibekkum@quicinc.com> wrote:
    >>>>>
    >>>>> Add ACTLR data table for SM8550 along with support for
    >>>>> same including SM8550 specific implementation operations.
    >>>>>
    >>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
    >>>>> ---
    >>>>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 89 ++++++++++++++++++++++
    >>>>>   1 file changed, 89 insertions(+)
    >>>>>
    >>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
    >>>>> index cb49291f5233..d2006f610243 100644
    >>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
    >>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
    >>>>> @@ -20,6 +20,85 @@ struct actlr_config {
    >>>>>          u32 actlr;
    >>>>>   };
    >>>>>
    >>>>> +/*
    >>>>> + * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the
    >>>>> + * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch
    >>>>> + * buffer). The remaining bits are implementation defined and vary across
    >>>>> + * SoCs.
    >>>>> + */
    >>>>> +
    >>>>> +#define PREFETCH_DEFAULT       0
    >>>>> +#define PREFETCH_SHALLOW       BIT(8)
    >>>>> +#define PREFETCH_MODERATE      BIT(9)
    >>>>> +#define PREFETCH_DEEP          (BIT(9) | BIT(8))
    >>>>
    >>>> I thin the following might be more correct:
    >>>>
    >>>> #include <linux/bitfield.h>
    >>>>
    >>>> #define PREFETCH_MASK GENMASK(9, 8)
    >>>> #define PREFETCH_DEFAULT FIELD_PREP(PREFETCH_MASK, 0)
    >>>> #define PREFETCH_SHALLOW FIELD_PREP(PREFETCH_MASK, 1)
    >>>> #define PREFETCH_MODERATE FIELD_PREP(PREFETCH_MASK, 2)
    >>>> #define PREFETCH_DEEP FIELD_PREP(PREFETCH_MASK, 3)
    >>>>
    >>>
    >>> Ack, thanks for this suggestion. Let me try this out using
    >>> GENMASK. Once tested, will take care of this in next version.
    >>
    >> FWIW the more typical usage would be to just define the named macros for the raw field values, then put the FIELD_PREP() at the point of use. However in this case that's liable to get pretty verbose, so although I'm usually a fan of bitfield.h, the most readable option here might actually be to stick with simpler definitions of "(0 << 8)", "(1 << 8)", etc. However it's not really a big deal either way, and I defer to whatever Dmitry and Konrad prefer, since they're the ones looking after arm-smmu-qcom the most :)
    > My 5 cents would be to just use the "common" style of doing this, so:
    >
    > #define ACTRL_PREFETCH GENMASK(9, 8)
    > #define PREFETCH_DEFAULT 0
    > #define PREFETCH_SHALLOW 1
    > #define PREFETCH_MODERATE 2
    > #define PREFETCH_DEEP 3
    >
    > and then use
    >
    > | FIELD_PREP(ACTRL_PREFETCH, PREFETCH_x)
    >
    > it can get verbose, but.. arguably that's good, since you really want
    > to make sure the right bits are set here
    >

    Thanks for the suggestion with these mods.
    Let me try out the suggested way and once tested will post this in next
    version.

    Thanks,
    Bibek

    > Konrad

    \
     
     \ /
      Last update: 2023-12-18 09:44    [W:3.449 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site