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 3160CC433F5 for ; Wed, 4 May 2022 12:08:03 +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:Reply-To: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:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Sm95IPh6hfO2iqaxKvJ8mUcaLHuCTZf0STdMLN0Cn/o=; b=WGrUByDg4FFcgT 9LFrJxvK6hhrf6ZzBnlWbEQfOLSne4FBvV8CouBLoi7qsLnvP64DReHO282dzuXKUpnuYJg4WdJc2 s/Ffs69Aw6VrWV/lgjjt9dfOiGR/QBzweUCntcAH2bDMjrHFZifgbzOU2rAwBtAPZijlwXmN930Pa ClYst4djCPoidG9qw7UhN2rHJSvB3iXPXs/L4/2YDHtvhXnhJr7sSfXzdwSz6NnOU6a3xliLt3tv4 Gj56tb6AJ1jFREB0sDyILLF5oApvjgI8QG+tz/MUkGZJoRV3lb7b+vaiGM6FrlQlbRXv1HkGmplXz HeVKTadFkAGIRww8Z7jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmDmL-00AdWv-6K; Wed, 04 May 2022 12:06:50 +0000 Received: from mail-qt1-x82a.google.com ([2607:f8b0:4864:20::82a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nmDm8-00AdQX-0e for linux-arm-kernel@lists.infradead.org; Wed, 04 May 2022 12:06:38 +0000 Received: by mail-qt1-x82a.google.com with SMTP id hf18so665909qtb.0 for ; Wed, 04 May 2022 05:06:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:date:from:to:cc:subject:message-id:reply-to:references :mime-version:content-disposition:in-reply-to; bh=+WNZryBXL+g7kwNzKGyTKWK4wg/wzwMgT6OB9tUEVx4=; b=ZhzXCEkMq4XwxySq3BgrlTremloLuIKnLA4pTWCqSN9GHXJ7dLD8mUJk/a6Cx4DV9W qtBExIMVKBAmXh6F1zj87sqgxLJJoNG2txnzswjsQGHmgWu/KYeYWE6K5H9gp7SW1sI5 FvlrNc8ComRabkEV3E0joBA8MMxDscVIJoHVfHhOSci22mtwHPfELk8iLcRGDJ15ELIO QV/ET9LIWtE+LIp4kymM9EWIX3H72WMXcj5bRrb0wTFn5gkJ4iPpugvtCjMT928AliNZ 7ElEpkBUxqZBip8N0hWXNku1ShOJ7TwDuPgcw64PDBIcHnngwNTweMqLJopbxEcd/iQG uu9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :reply-to:references:mime-version:content-disposition:in-reply-to; bh=+WNZryBXL+g7kwNzKGyTKWK4wg/wzwMgT6OB9tUEVx4=; b=el/HHnAUYPH+68vy6yIq1PMmabqCw4jHKHFkBCl+f/5ECuG2Nx9QgqhDCj2cqrIlmy cyvHx/y9G6aaVcXCRxovVEkF14bCkfZwnbckQ6XqQhKWKzPxkDcEzoX+9Pg+prwqmYva R8MxuTx05Jqpc5MXrLmRjVr7xVFytE+z6ajEFPGhIoHL3gQnpMgtim8rm8IZiQm/dE3o zc6evT+iPX5bvs/9/zrL0D7gh91vTXoPvhM4o7Fy1zTUZCGIWw7KLPKZrOXMuklJsnvq Y56qTBNHDYaPcMnUJiDdn/b/LvrkpICtUgfrX/JbLbCbryno9RCHGwTZrRs2nZZ1/ckT Tt1Q== X-Gm-Message-State: AOAM532UbH4pr9nTapCngBd8oV9LjM9sXBcs6LvfVzJfjrg4wwOjCvFR zi0s99sv+ixYECKFqmP34aW+Eh8U1grjrng= X-Google-Smtp-Source: ABdhPJy8E2ZGhOMqaubc9bI3GOFs28UeBhEN5Ik/qSGSpbMeojwEWqvAJa0IzwkfLKv2xoNBPuJkig== X-Received: by 2002:a05:622a:1ba7:b0:2f3:a08e:7c11 with SMTP id bp39-20020a05622a1ba700b002f3a08e7c11mr15849578qtb.10.1651665993525; Wed, 04 May 2022 05:06:33 -0700 (PDT) Received: from serve.minyard.net (serve.minyard.net. [2001:470:b8f6:1b::1]) by smtp.gmail.com with ESMTPSA id l184-20020a3789c1000000b0069fc13ce22esm7512348qkd.95.2022.05.04.05.06.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 May 2022 05:06:33 -0700 (PDT) Received: from minyard.net (unknown [IPv6:2001:470:b8f6:1b:149c:2dc6:c0ab:4341]) by serve.minyard.net (Postfix) with ESMTPSA id 322621800BD; Wed, 4 May 2022 12:06:32 +0000 (UTC) Date: Wed, 4 May 2022 07:06:31 -0500 From: Corey Minyard To: Quan Nguyen Cc: devicetree@vger.kernel.org, linux-aspeed@lists.ozlabs.org, Krzysztof Kozlowski , Andrew Jeffery , Benjamin Herrenschmidt , openbmc@lists.ozlabs.org, "Thang Q . Nguyen" , Brendan Higgins , linux-kernel@vger.kernel.org, Phong Vo , Wolfram Sang , Rob Herring , openipmi-developer@lists.sourceforge.net, Open Source Submission , linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org Subject: Re: [Openipmi-developer] [PATCH v7 1/3] ipmi: ssif_bmc: Add SSIF BMC driver Message-ID: <20220504120631.GE3767252@minyard.net> References: <20220422040803.2524940-1-quan@os.amperecomputing.com> <20220422040803.2524940-2-quan@os.amperecomputing.com> <20220423015119.GE426325@minyard.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220504_050636_139842_E3DBB8BB X-CRM114-Status: GOOD ( 34.28 ) 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: , Reply-To: minyard@acm.org 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 Wed, May 04, 2022 at 01:45:03PM +0700, Quan Nguyen via Openipmi-developer wrote: > > > > I seem to remember mentioning this before, but there is no reason to > > pack the structures below. > > > > The packed structure is because we want to pick the len directly from user > space without worry about the padding byte. > > As we plan not to use the .h file in next version, I still would like to use > packed structure internally inside ssif_bmc.c file. Packed doesn't matter for the userspace API. If you look at other structures in the userspace API, they are not packed, either. The compiler will do the right thing on both ends. > > > And second, the following is a userspace API structures, so it needs to > > be in its own file in include/uapi/linux, along with any supporting > > things that users will need to use. And your userspace code should be > > using that file. > > > > Meantime, I'd like not to use .h as I see there is no demand for sharing the > data structure between kernel and user space yet. But we may do it in the > future. If you have a userspace API, it needs to be in include/uapi/linux. You may not be the only user of this code. In fact, you probably won't be. You need to have a .h with the structures in it, you don't want the same structure in two places if you can help it. > > > > +struct ssif_msg { > > > + u8 len; > > > > Just to be 100% safe, it might be better to use a u16 on this. The spec > > sort of limits this to 255 bytes, but it also sort of leaves it open to > > be larger. > > > Yes, u8 only limited to 255 bytes and there is no space for future grow. Please make it a unsigned int for the length and __u8 for the data to give necessary flexibility. Thanks, -corey > > > > + u8 payload[MSG_PAYLOAD_LEN_MAX]; > > > +} __packed; > > > + > > > +struct ssif_part_buffer { > > > + u8 address; > > > + u8 smbus_cmd; > > > + u8 length; > > > + u8 payload[MAX_PAYLOAD_PER_TRANSACTION]; > > > + u8 pec; > > > + u8 index; > > > +} __packed; > > > + > > > +/* > > > + * SSIF internal states: > > > + * SSIF_READY 0x00 : Ready state > > > + * SSIF_START 0x01 : Start smbus transaction > > > + * SSIF_SMBUS_CMD 0x02 : Received SMBus command > > > + * SSIF_REQ_RECVING 0x03 : Receiving request > > > + * SSIF_RES_SENDING 0x04 : Sending response > > > + * SSIF_BAD_SMBUS 0x05 : Bad SMbus transaction > > > + */ > > > +enum ssif_state { > > > + SSIF_READY, > > > + SSIF_START, > > > + SSIF_SMBUS_CMD, > > > + SSIF_REQ_RECVING, > > > + SSIF_RES_SENDING, > > > + SSIF_ABORTING, > > > + SSIF_STATE_MAX > > > +}; > > > + > > > +struct ssif_bmc_ctx { > > > + struct i2c_client *client; > > > + struct miscdevice miscdev; > > > + int msg_idx; > > > + bool pec_support; > > > + /* ssif bmc spinlock */ > > > + spinlock_t lock; > > > + wait_queue_head_t wait_queue; > > > + u8 running; > > > + enum ssif_state state; > > > + /* Timeout waiting for response */ > > > + struct timer_list response_timer; > > > + bool response_timer_inited; > > > + /* Flag to identify a Multi-part Read Transaction */ > > > + bool is_singlepart_read; > > > + u8 nbytes_processed; > > > + u8 remain_len; > > > + u8 recv_len; > > > + /* Block Number of a Multi-part Read Transaction */ > > > + u8 block_num; > > > + bool request_available; > > > + bool response_in_progress; > > > + bool busy; > > > + bool aborting; > > > + /* Buffer for SSIF Transaction part*/ > > > + struct ssif_part_buffer part_buf; > > > + struct ssif_msg response; > > > + struct ssif_msg request; > > > +}; > > > + > > > +static inline struct ssif_bmc_ctx *to_ssif_bmc(struct file *file) > > > +{ > > > + return container_of(file->private_data, struct ssif_bmc_ctx, miscdev); > > > +} > > > +#endif /* __SSIF_BMC_H__ */ > > > -- > > > 2.35.1 > > > > > > > > > > _______________________________________________ > Openipmi-developer mailing list > Openipmi-developer@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openipmi-developer _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel