Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 10 Nov 2022 10:36:18 +0000
From:   Sudeep Holla <sudeep.holla@arm.com>
To:     Huisong Li <lihuisong@huawei.com>
Cc:     linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
        rafael@kernel.org, rafael.j.wysocki@intel.com,
        Sudeep Holla <sudeep.holla@arm.com>, wanghuiqiang@huawei.com,
        zhangzekun11@huawei.com, wangxiongfeng2@huawei.com,
        tanxiaofei@huawei.com, guohanjun@huawei.com, xiexiuqi@huawei.com,
        wangkefeng.wang@huawei.com, huangdaode@huawei.com
Subject: Re: [PATCH 2/3] ACPI: PCC: add check for platform interrupt
Message-ID: <20221110103618.3vuyfdhcebf7ewmo@bogus>
References: <20221110015034.7943-1-lihuisong@huawei.com>
 <20221110015034.7943-3-lihuisong@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221110015034.7943-3-lihuisong@huawei.com>
Precedence: bulk

On Thu, Nov 10, 2022 at 09:50:33AM +0800, Huisong Li wrote:
> PCC Operation Region driver senses the completion of command by interrupt
> way. If platform can not generate an interrupt when a command complete,
> the caller never gets the desired result. So let's reject the setup of the
> PCC address space on platform that do not support interrupt mode.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  drivers/acpi/acpi_pcc.c | 47 +++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_pcc.c b/drivers/acpi/acpi_pcc.c
> index 3e252be047b8..8efd08e469aa 100644
> --- a/drivers/acpi/acpi_pcc.c
> +++ b/drivers/acpi/acpi_pcc.c
> @@ -53,6 +53,7 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
>  	struct pcc_data *data;
>  	struct acpi_pcc_info *ctx = handler_context;
>  	struct pcc_mbox_chan *pcc_chan;
> +	static acpi_status ret;
>  
>  	data = kzalloc(sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -69,23 +70,35 @@ acpi_pcc_address_space_setup(acpi_handle region_handle, u32 function,
>  	if (IS_ERR(data->pcc_chan)) {
>  		pr_err("Failed to find PCC channel for subspace %d\n",
>  		       ctx->subspace_id);
> -		kfree(data);
> -		return AE_NOT_FOUND;
> +		ret = AE_NOT_FOUND;
> +		goto request_channel_fail;
>  	}
>

Your patch seems to be not based on the upstream.
Commit f890157e61b8 ("ACPI: PCC: Release resources on address space setup
failure path") has addressed it already.

>  	pcc_chan = data->pcc_chan;
> +	if (!pcc_chan->mchan->mbox->txdone_irq) {
> +		pr_err("This channel-%d does not support interrupt.\n",
> +		       ctx->subspace_id);
> +		ret = AE_SUPPORT;
> +		goto request_channel_fail;
> +	}

Indeed, I supported only interrupt case and this approach is better than
checking it in handler atleast until we add support for polling based
transfers in future(hope that never happens, but you never know)

-- 
Regards,
Sudeep