Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <61d2885a-50f9-419f-8473-10308baed493@quicinc.com>
Date:   Thu, 16 Nov 2023 11:39:49 +0530
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for
 SM8550
Content-Language: en-US
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
CC:     <will@kernel.org>, <robin.murphy@arm.com>, <joro@8bytes.org>,
        <a39.skl@gmail.com>, <quic_pkondeti@quicinc.com>,
        <quic_molvera@quicinc.com>, <linux-arm-msm@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>, <qipl.kernel.upstream@quicinc.com>
References: <20231114135654.30475-1-quic_bibekkum@quicinc.com>
 <20231114135654.30475-3-quic_bibekkum@quicinc.com>
 <CAA8EJpr1NzqiuNVZ0YcLpJ=yeOYFbLouAFgN9VMOiKpmoGVdtQ@mail.gmail.com>
 <7edad996-f148-42d5-8e72-0334d3b9101f@quicinc.com>
 <CAA8EJpqzEvC-LEjJrOk6Ed8utFNnqgP=qkb0mq-dOfMZd5ij4Q@mail.gmail.com>
 <dba223a1-0456-40b8-b295-ea960867dcf9@quicinc.com>
 <CAA8EJpqvXb_Zdmq0O0KEA8mn5UkSriCZ02w16dts7Gd84R30oQ@mail.gmail.com>
 <aebdf7f6-48b7-43d3-95af-83859ddc593f@quicinc.com>
 <be9675a6-a01e-4cf7-95b3-174220d4a042@linaro.org>
From:   Bibek Kumar Patro <quic_bibekkum@quicinc.com>
In-Reply-To: <be9675a6-a01e-4cf7-95b3-174220d4a042@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 11/15/2023 10:12 PM, Konrad Dybcio wrote:
> 
> 
> On 11/15/23 13:49, Bibek Kumar Patro wrote:
>>
>>
>> On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote:
>>> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro
>>> <quic_bibekkum@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro
>>>>> <quic_bibekkum@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, 14 Nov 2023 at 15:57, 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 | 92 
>>>>>>>> +++++++++++++++++++++-
>>>>>>>>     1 file changed, 88 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
>>>>>>>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>>> index 578c662c7c30..0eaf6f2a2e49 100644
>>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>>> @@ -25,6 +25,70 @@ struct actlr_data {
>>>>>>>>            u32 actlr;
>>>>>>>>     };
>>>>>>>>
>>>>>>>> +#define PRE_FETCH_1    0
>>>>>>>> +#define PRE_FETCH_2    BIT(8)
>>>>>>>> +#define PRE_FETCH_3    (BIT(9) | BIT(8))
>>>>>>>
>>>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And
>>>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / 
>>>>>>> count
>>>>>>> or just dummy names?
>>>>>>>
>>>>>>
>>>>>> No,these are not real numbers, but prefetch settings for a particular
>>>>>> perfect configuration.
>>>>>
>>>>> Then I'd ask for some better names or descriptions.
>>>>>
>>>>
>>>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name
>>>> in the following case. Would it be okay to use this name instead?
>>>
>>> Not really.
>>>
>>
>> Any suggestion you have in mind, if not this nomenclature?
> Dmitry's concern seems to be that you provide:
> 
> PRE_FETCH_1 /* prefetcher with settings preset no. 1 */
> PRE_FETCH_2 /* prefetcher with settings preset no. 2 */
> PRE_FETCH_3 /* prefetcher with settings preset no. 3 */
> 
> whereas it would be both useful and interesting to see what these
> settings mean, i.e. what differences there are between all of
> these presets.
> 

Ah, okay got it now from Dimitry and yours' response.
But we exactly won't be able to reveal what each of these settings
mean, as this might risk of revealing IP as ACTLR bits are
implementation defined (except CPRE and CMTLB) which other SoC vendors
might be using it in different manner(or different purpose) in their
downstream implementation.
We can name it like (e.g PREFETCH_DISABLE, PREFETCH_SHALLOW,
PREFETCH_DEEP) to indicate the behaviour, but won't be exactly
name/describe it to explain what it does with a particular setting.

> Konrad