From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 849D6CD610E for ; Mon, 9 Oct 2023 16:50:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=RvKSwK15oU/IY2UPXfUg+wq0zekaxEmvazsfUPWvK70=; b=VddxViDaRy8Fxw 3lfMgTkOzgHVBiMJJj9bNOYDhHjOJIaH+UcGWg2wElJZ+7qVhMqCbXtAoETcv+nhY+gArj4EHS0Is EffeLfSJNZTId5VZrbSh2YBVRBh/47edEvP2xVNutiIPUyZmTy/LaHF/dxDAesUgq/sBZJsONOjYL +N62XqfJq2hXwsbkFzNJbEzuBdwtDZeKmNzJw5BynftLlGUowfMMxZN9gfYpx8kRqtbaSqIPBlZbJ bp+QRMoCq1hgrw6VIzCxHqxc69mnj8980n1Vz0/hBTZM8qio3T/k30O1dfvjeLsM9vdnPy4KoTbt9 ZXSeI0eEx3q9vt84FdhA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qptRm-00BDwa-2f; Mon, 09 Oct 2023 16:49:34 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qptRi-00BDvx-24 for linux-arm-kernel@lists.infradead.org; Mon, 09 Oct 2023 16:49:33 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D6A811FB; Mon, 9 Oct 2023 09:50:08 -0700 (PDT) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4ED7F3F5A1; Mon, 9 Oct 2023 09:49:27 -0700 (PDT) Date: Mon, 9 Oct 2023 17:49:25 +0100 From: Cristian Marussi To: Sudeep Holla Cc: linux-arm-kernel@lists.infradead.org, andersson@kernel.org, Nikunj Kela Subject: Re: [PATCH] firmware: arm_scmi: Convert u32 to unsigned long to align with arm_smccc_1_1_invoke() Message-ID: References: <20231009152049.1428872-1-sudeep.holla@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20231009152049.1428872-1-sudeep.holla@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231009_094930_748960_879A6D76 X-CRM114-Status: GOOD ( 22.13 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 09, 2023 at 04:20:49PM +0100, Sudeep Holla wrote: > All the parameters to arm_smccc_1_1_invoke() are unsigned long which > aligns well on both 32-bit and 64-bit Arm based platforms. Let us store > all the members in the structure scmi_smc used as the parameters to the > arm_smccc_1_1_invoke() call as unsigned long. > > Cc: Cristian Marussi > Signed-off-by: Sudeep Holla > --- > drivers/firmware/arm_scmi/smc.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c > index c193516a254d..8eba60a41975 100644 > --- a/drivers/firmware/arm_scmi/smc.c > +++ b/drivers/firmware/arm_scmi/smc.c > @@ -60,9 +60,9 @@ struct scmi_smc { > struct mutex shmem_lock; > #define INFLIGHT_NONE MSG_TOKEN_MAX > atomic_t inflight; > - u32 func_id; > - u32 param_page; > - u32 param_offset; > + unsigned long func_id; > + unsigned long param_page; > + unsigned long param_offset; > }; > > static irqreturn_t smc_msg_done_isr(int irq, void *data) > @@ -211,8 +211,6 @@ static int smc_send_message(struct scmi_chan_info *cinfo, > { > struct scmi_smc *scmi_info = cinfo->transport_info; > struct arm_smccc_res res; > - unsigned long page = scmi_info->param_page; > - unsigned long offset = scmi_info->param_offset; > > /* > * Channel will be released only once response has been > @@ -222,8 +220,8 @@ static int smc_send_message(struct scmi_chan_info *cinfo, > > shmem_tx_prepare(scmi_info->shmem, xfer, cinfo); > > - arm_smccc_1_1_invoke(scmi_info->func_id, page, offset, 0, 0, 0, 0, 0, > - &res); > + arm_smccc_1_1_invoke(scmi_info->func_id, scmi_info->param_page, > + scmi_info->param_offset, 0, 0, 0, 0, 0, &res); > > /* Only SMCCC_RET_NOT_SUPPORTED is valid error code */ > if (res.a0) { > -- Looking at where scmi_info->param_page and scmi_info->param_offset are set with the SHMEM_PAGE/OFFSET macros, I dont get what is the supposed meaning of _UL macros in the original: #define SHMEM_SHIFT 12 #define SHMEM_PAGE(x) (_UL((x) >> SHMEM_SHIFT)) given it is used as: scmi_info->param_page = SHMEM_PAGE(res.start); which indeed looking at the pre-processor output yields (somehow) just a simple: scmi_info->param_page = ((((res.start) >> 12UL))); ...this was introduced in a previous patch, though: 5f2ea10a808ae firmware: arm_scmi: Augment SMC/HVC to allow optional parameters ..not sure if there are other deeper intents here... But anyway, with this patch any form of casting is probably not needed anymore since you'll have a: unsigned long (u32 or u64) <<--- res.start (u32 or u64) ...not sure if static analyzers and/or compilers would complain/warn these days. Other than this, LGTM: Reviewed-by: Cristian Marussi Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel