Received-SPF: pass (google.com: domain of linux-kernel+bounces-22062-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <ec31fafa-b912-454a-8b64-e0593911aaf2@quicinc.com>
Date: Wed, 10 Jan 2024 16:46:46 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 3/5] iommu/arm-smmu: introduction of ACTLR for custom
 prefetcher settings
To: Pavan Kondeti <quic_pkondeti@quicinc.com>
CC: <will@kernel.org>, <robin.murphy@arm.com>, <joro@8bytes.org>,
        <dmitry.baryshkov@linaro.org>, <konrad.dybcio@linaro.org>,
        <jsnitsel@redhat.com>, <quic_bjorande@quicinc.com>, <mani@kernel.org>,
        <quic_eberman@quicinc.com>, <robdclark@chromium.org>,
        <u.kleine-koenig@pengutronix.de>, <robh@kernel.org>,
        <vladimir.oltean@nxp.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: <20240109114220.30243-1-quic_bibekkum@quicinc.com>
 <20240109114220.30243-4-quic_bibekkum@quicinc.com>
 <2ad70157-27d1-41df-8866-c226af690cf6@quicinc.com>
Content-Language: en-US
From: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
In-Reply-To: <2ad70157-27d1-41df-8866-c226af690cf6@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/10/2024 9:36 AM, Pavan Kondeti wrote:
> On Tue, Jan 09, 2024 at 05:12:18PM +0530, Bibek Kumar Patro wrote:
>>   static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>>   		struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>>   {
>> +	struct arm_smmu_device *smmu = smmu_domain->smmu;
>> +	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> +	const struct actlr_variant *actlrvar;
>> +	int cbndx = smmu_domain->cfg.cbndx;
>>   	struct adreno_smmu_priv *priv;
>>
>>   	smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>> @@ -248,6 +285,16 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>>   	priv->set_stall = qcom_adreno_smmu_set_stall;
>>   	priv->resume_translation = qcom_adreno_smmu_resume_translation;
>>
>> +	if (qsmmu->data->actlrvar) {
>> +		actlrvar = qsmmu->data->actlrvar;
>> +		for (; actlrvar->io_start; actlrvar++) {
>> +			if (actlrvar->io_start == smmu->ioaddr) {
>> +				qcom_smmu_set_actlr(dev, smmu, cbndx, actlrvar->actlrcfg);
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>>   	return 0;
>>   }
>>
>> @@ -274,6 +321,21 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
>>   static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>>   		struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>>   {
>> +	struct arm_smmu_device *smmu = smmu_domain->smmu;
>> +	struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> +	const struct actlr_variant *actlrvar;
>> +	int cbndx = smmu_domain->cfg.cbndx;
>> +
>> +	if (qsmmu->data->actlrvar) {
>> +		actlrvar = qsmmu->data->actlrvar;
>> +		for (; actlrvar->io_start; actlrvar++) {
>> +			if (actlrvar->io_start == smmu->ioaddr) {
>> +				qcom_smmu_set_actlr(dev, smmu, cbndx, actlrvar->actlrcfg);
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
> 
> This block and the one in qcom_adreno_smmu_init_context() are exactly
> the same. Possible to do some refactoring?
> 

I will check if this repeated blocks can be accomodated this into 
qcom_smmu_set_actlr function if that would be fine.

>>   	smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>>   	return 0;
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> index f3b91963e234..29d26dfa2ed9 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> @@ -1,6 +1,6 @@
>>   /* SPDX-License-Identifier: GPL-2.0-only */
>>   /*
>> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>
> 
> It should be 2022-2023 .
> 

Ack

>>   #ifndef _ARM_SMMU_QCOM_H
>> @@ -24,8 +24,17 @@ struct qcom_smmu_config {
>>   	const u32 *reg_offset;
>>   };
>>
>> +struct actlr_config;
>> +
>> +struct actlr_variant {
>> +	const struct actlr_config *actlrcfg;
>> +	const resource_size_t io_start;
>> +};
>> +
>>   struct qcom_smmu_match_data {
>> +	const struct actlr_variant *actlrvar;
>>   	const struct qcom_smmu_config *cfg;
>> +	const int num_smmu;
>>   	const struct arm_smmu_impl *impl;
>>   	const struct arm_smmu_impl *adreno_impl;
>>   };
> 
> qcom_smmu_match_data::num_smmu needs cleanup.
> 

Ack, thanks for pointing this out.

Thanks & regards,
Bibek

> Thanks,
> Pavan