Messages in this thread | | | Date | Mon, 18 Dec 2023 11:47:30 +0530 | Subject | Re: [PATCH v4 3/5] iommu/arm-smmu: add ACTLR data and support for SM8550 | From | Bibek Kumar Patro <> |
| |
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
| |