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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A2FBC3DA7F for ; Mon, 12 Aug 2024 20:16:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB8D56B00B6; Mon, 12 Aug 2024 16:16:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A67C56B00B7; Mon, 12 Aug 2024 16:16:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 92F796B00BA; Mon, 12 Aug 2024 16:16:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 74D336B00B6 for ; Mon, 12 Aug 2024 16:16:55 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 16525140354 for ; Mon, 12 Aug 2024 20:16:55 +0000 (UTC) X-FDA: 82444701990.11.A6C949E Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf29.hostedemail.com (Postfix) with ESMTP id 4EBF3120009 for ; Mon, 12 Aug 2024 20:16:53 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=W5QZMKWa; spf=pass (imf29.hostedemail.com: domain of andi.shyti@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=andi.shyti@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723493802; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=OAW1Ox36NJHTfdfH+86QDc46ihtJsk4FjW5kLA6SX9E=; b=JBqWE/zD0ZszjeQxvF/7P102qJSVu/A4+P3juktq4nEAEzRLl/2VyABT2BD0OALVElmA0S osT7M8qtc9YeL7d4H0xQZoeckBJ/05c5Tzx+Qj0Gf+eWGEaZFLDxQqJ9CL4QNg/G0qXXl3 x64aZ0SMJiz0Xz64dcVaYCVhwvgT6Vw= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=W5QZMKWa; spf=pass (imf29.hostedemail.com: domain of andi.shyti@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=andi.shyti@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723493802; a=rsa-sha256; cv=none; b=1yKQ/wDsc08mYN0SzAeZQVeH2/hm0GzKetlC3wxwO2hH/y5ZrWoASNpFVDa3EGBBfD+eJ7 pu8Aq5ELaVvbP6U+owsu/DCoWjDGGGwYaTzu08XO5D89wJCuzrrhPJaehKABQnPwP7FT5c hbSWu9lEahEOuBorgy4m3pKOVJchPcU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 48E17608CC; Mon, 12 Aug 2024 20:16:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 30DBAC32782; Mon, 12 Aug 2024 20:16:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723493812; bh=LIjLr0XWh6AeIzZ0yZULxR8mFjgXuYMZLKGdmyAkiow=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=W5QZMKWa43vbq77u9uCD03JinbZbWZV1Jo7M9DItTsyB4CC2/w6NcPq8OuMaSdtPc QeRkroMmtuvzL1NuKaFvcUNkIt6tKVQyGSVHTAICnSn26q4PsyL4jrfTuXhT8uYxr6 E3aXunhJ7PeZ0lMnqee3MxSUytur78RH1Eylm/1Ndh0KHr/eIm7nF7VuxJ9p3ga95P 4Ck1iQRkw9N+1NLYpY0ksnGn5Qq3WAB9fYXomvvFiesTxoOzpW7T2bzNSssqTA5aL2 O5+5MKENjT4XhaXrZ9lb3JutSLd3oByTwQBd30pXzYSv/nMcJ3nfv8jzjTU8YM1e+R x/Ht6Hrnm1MJw== Date: Mon, 12 Aug 2024 21:16:47 +0100 From: Andi Shyti To: Mary Strodl Cc: Mary Strodl , linux-kernel@vger.kernel.org, akpm@linux-foundation.org, urezki@gmail.com, hch@infradead.org, linux-mm@kvack.org, lee@kernel.org, linux-i2c@vger.kernel.org, s.hauer@pengutronix.de, christian.gmeiner@gmail.com Subject: Re: [PATCH v3 1/2] x86: Add basic support for the Congatec CGEB BIOS interface Message-ID: References: <20240808183527.3950120-1-mstrodl@csh.rit.edu> <20240808183527.3950120-2-mstrodl@csh.rit.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Stat-Signature: qxkmo883x6sscnezbq1gif36e1jokubh X-Rspamd-Queue-Id: 4EBF3120009 X-Rspamd-Server: rspam11 X-HE-Tag: 1723493813-544339 X-HE-Meta: U2FsdGVkX193m2xhBSjsD597xrG+/CnupzYnpSLKV7Zr96JirqVXJATqs96yRd4Syxi+4HFnyyUr75cmv4cyDkT6o2uTNnZdRe6a5NrMvs/bqdrYb6uIgyov4AFf+AqnlVfjfwqKhBc5AWqc7+Q7SRSeo9cLh7PXrwjbqz5utGAsdFQXqmGbgXbsWMrkdaejtNlD510kdOUv8jYoG4efDfKoPfQsl4HPKq3KamJUlb6eD5pi/eaG5mfbxqtfiR9PcYPOWMnrv0ySdrl+u/Iw8uYwPIkHqDSdLTDFA7dUijp/dIgWZ67y1loQqwj3/7MZP9l3CL1jUZno+ADZkISFd09Gi9I0pDnjWJkB2qfxhrlLFiuvmvA8vBJF2/MjKXgW8Mr+dryV22uqf+/WDHAvAowXDj1RPoieWHRBKcRCwz/C12PFX6FzkQBNE4iZcOO5R7hbb9VE4+0Zi2sUPIq1qPHEORa3wTICiMH1LLJack+aEN26xJOluuctkIF8svXwn5paTQYfRPBr3kZSwApluVhbUsGWWnqmSce43RodvYVoP/eCdSt+p6jKloHdtaNXmzJgAKmCaGgWghBbFfiJvS/OimzcekgbTzevRKCV8ugkdP3ZphiowWFLnCBpC8qALLRuUxewa/+KY5hElxlcIFb/WFMteP9OynnghYCSS09yLBOqRfGvNXzWB8hk9entXhWb3pHQ3lUjteGrJNJ5UtHoN+uKXE4WQSGsW+JJAa5NK8NLou1BXi87aZc/rTstZ98/iPIyZexcM+eIb7v3PdFlPBHl5QeYVzsdMRTEO6T1spGqHuEk/JRkVeWjTEmhowD3ogtI2iUOfV7GmWAolbg4XuVryVY7+Nm3fXLt/20MbUMDiXnPFsI5B2x+askD2GZPtk28sRPdRdkbSBMLVQxDPt4PA3DicQZbLva5On+fWhwWvyctMgewYKd5iltJGqVMEEKXbfVjLvCsmnM og6TwAAj /JKXd4oTtC+ku+iA/A4H4weS5Rhmp6f6FdNDT6C1OARe+xIXIhfvGwQvUDn1kk7prjXfHfvXOV525mDS6gPqmEV28qMm8vsELfxR7+aQAKUBsgZwXR7avy+gfjAYXgoaguHNYN84kN5B7e8YRD4hFZ61TQYkzyfXaf3rzn4Fe3Zt7HgzQlWYuI+JxyO3LLkxx9j5bFVaZUXpj7qf/7SL8T2PJl5avIw/tJXLBmYPdRRMqngkXfl/1gRCwMbD5IorgmmzUn1UMOa/7Xl3pctlH4am/Dkqsejg0qj9/4lkpmjOXiRA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Mary, > > I'm counting too many global variables here and too many global > > structure definitions. It might get a bit hard to follow. > > > > I'd suggest to simplify this part as much as possible and keep > > everything in as less structures as possible (ideally just one). > > > > Please make local as many variables as you can. > > I don't disagree with you here. Unfortunately everything between > the pack(push, 4) and pack(pop) needs to stay, since those are > the BIOS's ABI. I'd be open to putting them in a header file or > something though, but I wasn't sure what convention was on stuff like that. > > What do you think? I think moving the BIOS ABI and maybe the over-the-wire > types (struct cgeb_request and friends) out would probably make it much > easier to follow. mine was a guess, actually... if you see that my comment doesn't fit, feel free to ignore it. As I said I didn't look in details all the structures and their dependencies. > > > + > > > + /* Wait for a response to the request */ > > > + err = wait_for_completion_interruptible_timeout( > > > + &req->done, msecs_to_jiffies(20000)); > > > + if (err == 0) { > > > + pr_err("CGEB: Timed out running request of type %d!\n", > > > + msg.type); > > > + err = -ETIMEDOUT; > > > > just "return -ETIMEDOUT;" and you can cleanup the code below > Here's where I landed on that: > > /* Wait for a response to the request */ > err = wait_for_completion_interruptible_timeout( > &req->done, msecs_to_jiffies(20000)); > if (err == 0) { > pr_err("CGEB: Timed out running request of type %d!\n", > msg.type); > return -ETIMEDOUT; > } else if (err < 0) { > return err; > } else { you don't ened anymore for this else, at the end, but this looks correct. Thanks, Andi