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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A8F4C433EF for ; Thu, 3 Feb 2022 10:34:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234355AbiBCKew (ORCPT ); Thu, 3 Feb 2022 05:34:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60706 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349720AbiBCKev (ORCPT ); Thu, 3 Feb 2022 05:34:51 -0500 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 76916C061714 for ; Thu, 3 Feb 2022 02:34:51 -0800 (PST) Received: by mail-pf1-x42b.google.com with SMTP id n32so1838654pfv.11 for ; Thu, 03 Feb 2022 02:34:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YUz06pZnKz52lqRpYNAE/X72JjI+Vf6k4/BCrzBH3FU=; b=UiyejmSz8yF32Lluhet7PwBZMrAziRCNepEXHIRQlOebPKYaQ8xt4lol4zyxxQHa/Q CNWjJSte15D181ytzgj3PLcj5+ELoliMZD89UtGx38yFcahG0MiDDJA1doOLInEAr8ah LsRKIOE+IfpQfIHa+RxnrPufXugbm5olIQbixPUcEQX7WZtpSoYxj73GznqwAW+gNCdc Dd2IVudc1Nzg56St5hqWcSKMmDMGlEyuZgxCHwqAeA+4sxNcuS+sftySZMFJraqZCuOC 8ruvRCgfz9JW22ScJNYrirO4KHyEi988ltI9VsZdp6UdpD62mZ9QRBjiHNawA3Gh65Uf xQBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YUz06pZnKz52lqRpYNAE/X72JjI+Vf6k4/BCrzBH3FU=; b=01cEhrRPkkhc0riVwV1JzZQwzIT9Q1DoMfiuy5o1gFjn69OMjN8sJ0tQQhgMW8X5dF s3mv+WsKlhfc9ODL37duBoEEOTXn/v0ye/0n6FQPqyVuvFI2WJiKwytvBgANZ2NEBlsa ysDoT/OjGQys8qumBwwaRsR/8tSml1EPZjX/EsO0KjRYzxwxBbBt0fSLAWg0GjBC6l6q 2CpHQ326T1FxzfnEALmgd1zH5hePe7dW8RrxSacjzsecsjoOp1b+rKLSlxJj1j5c0FP8 RFGJBG+0VEtIOWNby1y8j9rHcFH8nsApuwABuINAyFwWM/I4I3hmpphYOE9JTy+pnzNt daSQ== X-Gm-Message-State: AOAM533RjoVKADbauqhOCgFapiAoV0bHozutxgf6IleSua7rsMzPSBoq TAdZ3eUF8vWKa8zOnhL4e2wg X-Google-Smtp-Source: ABdhPJzrwgJPFeeQitXNGNkm/WAGb5B3nYHnSFQ8Z1cdKI47rjuG824pm5t6p2DPKTjVrvwLezE0Fw== X-Received: by 2002:a63:5460:: with SMTP id e32mr27756006pgm.330.1643884490874; Thu, 03 Feb 2022 02:34:50 -0800 (PST) Received: from thinkpad ([117.217.179.179]) by smtp.gmail.com with ESMTPSA id m21sm29694312pfk.26.2022.02.03.02.34.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Feb 2022 02:34:50 -0800 (PST) Date: Thu, 3 Feb 2022 16:04:44 +0530 From: Manivannan Sadhasivam To: Alex Elder Cc: mhi@lists.linux.dev, hemantk@codeaurora.org, bbhatt@codeaurora.org, quic_jhugo@quicinc.com, vinod.koul@linaro.org, bjorn.andersson@linaro.org, dmitry.baryshkov@linaro.org, skananth@codeaurora.org, vpernami@codeaurora.org, vbadigan@codeaurora.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/20] bus: mhi: Make mhi_state_str[] array static const and move to common.h Message-ID: <20220203103444.GB6298@thinkpad> References: <20211202113553.238011-1-manivannan.sadhasivam@linaro.org> <20211202113553.238011-4-manivannan.sadhasivam@linaro.org> <0d850b27-5684-2ecf-fc96-3258c0462d3d@ieee.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0d850b27-5684-2ecf-fc96-3258c0462d3d@ieee.org> Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Wed, Jan 05, 2022 at 06:22:59PM -0600, Alex Elder wrote: > On 12/2/21 5:35 AM, Manivannan Sadhasivam wrote: > > mhi_state_str[] array could be used by MHI endpoint stack also. So let's > > make the array as "static const" and move it inside the "common.h" header > > so that the endpoint stack could also make use of it. Otherwise, the > > structure definition should be present in both host and endpoint stack and > > that'll result in duplication. > > > > Signed-off-by: Manivannan Sadhasivam > > This result in common source code (which is good), but it will be > duplicated in everything that includes this file. > > Do you have no common code, available to both the endpoint and host? > You could (in drivers/bus/mhi/common.c, for example). > > If you don't, I have a different suggestion, below. It does > basically the same thing you're doing here, but I much prefer > duplicating an inline function than a data structure. > > > --- > > drivers/bus/mhi/common.h | 13 ++++++++++++- > > drivers/bus/mhi/host/init.c | 12 ------------ > > 2 files changed, 12 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h > > index 0f4f3b9f3027..2ea438205617 100644 > > --- a/drivers/bus/mhi/common.h > > +++ b/drivers/bus/mhi/common.h > > @@ -174,7 +174,18 @@ struct mhi_cmd_ctxt { > > __u64 wp __packed __aligned(4); > > }; > > -extern const char * const mhi_state_str[MHI_STATE_MAX]; > > +static const char * const mhi_state_str[MHI_STATE_MAX] = { > > + [MHI_STATE_RESET] = "RESET", > > + [MHI_STATE_READY] = "READY", > > + [MHI_STATE_M0] = "M0", > > + [MHI_STATE_M1] = "M1", > > + [MHI_STATE_M2] = "M2", > > + [MHI_STATE_M3] = "M3", > > + [MHI_STATE_M3_FAST] = "M3 FAST", > > + [MHI_STATE_BHI] = "BHI", > > + [MHI_STATE_SYS_ERR] = "SYS ERROR", > > +}; > > + > > #define TO_MHI_STATE_STR(state) ((state >= MHI_STATE_MAX || \ > > !mhi_state_str[state]) ? \ > > "INVALID_STATE" : mhi_state_str[state]) > > You could easily and safely define this as an inline function instead. > Sounds good! > #define MHI_STATE_CASE(x) case MHI_STATE_ ## x: return #x > static inline const char *mhi_state_string(enum mhi_state state) > { > switch(state) { > MHI_STATE_CASE(RESET); > MHI_STATE_CASE(READY); > MHI_STATE_CASE(M0); > MHI_STATE_CASE(M1); > MHI_STATE_CASE(M2); > MHI_STATE_CASE(M3_FAST); > MHI_STATE_CASE(BHI); > MHI_STATE_CASE(SYS_ERR); > default: return "(unrecognized MHI state)"; > } > } > #undef MHI_STATE_CASE I've used the below one: static inline const char * const mhi_state_str(enum mhi_state state) { switch(state) { case MHI_STATE_RESET: return "RESET"; case MHI_STATE_READY: return "READY"; case MHI_STATE_M0: return "M0"; case MHI_STATE_M1: return "M1"; case MHI_STATE_M2: return"M2"; case MHI_STATE_M3: return"M3"; case MHI_STATE_M3_FAST: return"M3 FAST"; case MHI_STATE_BHI: return"BHI"; case MHI_STATE_SYS_ERR: return "SYS ERROR"; default: return "Unknown state"; } }; Thanks, Mani > > -Alex > > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > > index 5aaca6d0f52b..fa904e7468d8 100644 > > --- a/drivers/bus/mhi/host/init.c > > +++ b/drivers/bus/mhi/host/init.c > > @@ -44,18 +44,6 @@ const char * const dev_state_tran_str[DEV_ST_TRANSITION_MAX] = { > > [DEV_ST_TRANSITION_DISABLE] = "DISABLE", > > }; > > -const char * const mhi_state_str[MHI_STATE_MAX] = { > > - [MHI_STATE_RESET] = "RESET", > > - [MHI_STATE_READY] = "READY", > > - [MHI_STATE_M0] = "M0", > > - [MHI_STATE_M1] = "M1", > > - [MHI_STATE_M2] = "M2", > > - [MHI_STATE_M3] = "M3", > > - [MHI_STATE_M3_FAST] = "M3 FAST", > > - [MHI_STATE_BHI] = "BHI", > > - [MHI_STATE_SYS_ERR] = "SYS ERROR", > > -}; > > - > > const char * const mhi_ch_state_type_str[MHI_CH_STATE_TYPE_MAX] = { > > [MHI_CH_STATE_TYPE_RESET] = "RESET", > > [MHI_CH_STATE_TYPE_STOP] = "STOP", > > >