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 X-Spam-Level: X-Spam-Status: No, score=-14.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DB60C2B9F7 for ; Mon, 24 May 2021 22:54:03 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 1CE08613D2 for ; Mon, 24 May 2021 22:54:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CE08613D2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=aj.id.au Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:Subject:Cc:To:From:Date:References: In-Reply-To:Message-Id:Mime-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TmY+obN7dKMHL22zmoOwIpdkYYELshI3cchOfLnMkc8=; b=o8C3kjs/+5pn3u n4NRkvDrS1E0EtbVzKXgWbvQx5m4Y9cmJWNX//t25L11nj7tDkjMseQfV2QcdRKewAkx+YHD2UqP1 ui12eazWkStsF7dmNQ/Wlf69qBKYlkNs1ogawPHoWtwRLrVHphvU49euSD8O++sl1IEVBSl1kByS4 KrQ/e+Hi8JFnDbipOCP4TdJBLjgNuCk6f/Z1ZyO8XZHaojTzXo+k6Qe9nHdMy5Fm8MV/kvA4s6dbb jX6XNYF88PQeIWXlshG8QUUHKMVu3w7prHXKFzvcJ3ZQzLf2pDzn8rb+EtF5D4YTQ0gspTupn7Xtb ypSsOx9jvZj3C7zoy7Zg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1llJQQ-002DSd-To; Mon, 24 May 2021 22:51:56 +0000 Received: from new2-smtp.messagingengine.com ([66.111.4.224]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lkyr6-000d83-H4 for linux-arm-kernel@lists.infradead.org; Mon, 24 May 2021 00:54:06 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id C31455802BD; Sun, 23 May 2021 20:54:02 -0400 (EDT) Received: from imap2 ([10.202.2.52]) by compute3.internal (MEProxy); Sun, 23 May 2021 20:54:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h= mime-version:message-id:in-reply-to:references:date:from:to:cc :subject:content-type; s=fm2; bh=c1N+jbjenOjR/XYKJ9JrzDDr2ArT8jH cPPYVtFN0+8s=; b=ok68MILGrCZHwK/EEaDToidzeAy70JVtB9FjJ/pwDgHMLPY ovb+ac2A0hKcALq3BTdinuwCUU4tFEwGrOvVUL4mgilE0fbIeLVIYkvTSg/A3C9A isVLljZCWMdha3ynTPS17MuqSVPwWl+xJfQt4IZm5lrj3u6/bDh1GWIUQBVKySVd 7ERl6fYx9Hzwm7OyafapAf+HBqlPX9FYAxNvCWVQ9ffjN0rYZxPnzQvHDbFhf3R7 IkCLkHtnZMQBtAnVcG+H+5GnhaCsHT3HniEb+iFzns1yaffCeyg6si9NuN84TU7r 3z366tuQSQtEi+GBbH7tKxItf/Q+Fdx23+XX7rw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=c1N+jb jenOjR/XYKJ9JrzDDr2ArT8jHcPPYVtFN0+8s=; b=sPFszQINYn+1MtITcFCPyx lNphgTAT0DJen+WqjLY3YH7HctWzuSJZ3zFTeSqLVuWZDYrW/idxoBb2M6LPgQEt ki/8+/Rk5S3eboxuGAYSLYMeGt4FwnCgkj+pOAJmlbNiPSPABGZshOSo60atWGdN cG0D6RtmqjLAFdmo/kORMxRpta5VgktFkB9TRMbfmX0piFWDr6uIDOQAVJbV4LjE DzJ46YD4tCg8RGGDBhp/tGiEQT+XdDLiX7JE1q4DwXdCPcoKSS4zY+wSpDL+idNh R1IVg/tHltd7L39TDpOgOOuq+WjNKHaAYxtN9BElPSGpkJQYPt1F36fzw4/0Bt1g == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdejkedgtdelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvffutgesthdtredtreerjeenucfhrhhomhepfdetnhgu rhgvficulfgvfhhfvghrhidfuceorghnughrvgifsegrjhdrihgurdgruheqnecuggftrf grthhtvghrnhepuddttdekueeggedvtddtueekiedutdfguedutdefieeuteefieelteet vddthfeinecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomh eprghnughrvgifsegrjhdrihgurdgruh X-ME-Proxy: Received: by mailuser.nyi.internal (Postfix, from userid 501) id A081EA004B1; Sun, 23 May 2021 20:54:01 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.5.0-alpha0-448-gae190416c7-fm-20210505.004-gae190416 Mime-Version: 1.0 Message-Id: <79f3c6d1-1f74-46ec-99a0-37faf11517b6@www.fastmail.com> In-Reply-To: <20210521171412.GI2921206@minyard.net> References: <20210510054213.1610760-1-andrew@aj.id.au> <20210510054213.1610760-6-andrew@aj.id.au> <20210521171412.GI2921206@minyard.net> Date: Mon, 24 May 2021 10:23:36 +0930 From: "Andrew Jeffery" To: "Corey Minyard" Cc: openipmi-developer@lists.sourceforge.net, openbmc@lists.ozlabs.org, devicetree@vger.kernel.org, "Tomer Maimon" , linux-aspeed@lists.ozlabs.org, "Avi Fishman" , "Patrick Venture" , linux-kernel@vger.kernel.org, "Tali Perry" , "Rob Herring" , "Chia-Wei, Wang" , linux-arm-kernel@lists.infradead.org, "Benjamin Fair" , "Arnd Bergmann" , "Zev Weiss" Subject: =?UTF-8?Q?Re:_[PATCH_v3_05/16]_ipmi:_kcs=5Fbmc:_Turn_the_driver_data-str?= =?UTF-8?Q?uctures_inside-out?= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210523_175404_661802_AD294B5B X-CRM114-Status: GOOD ( 32.26 ) 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 Sat, 22 May 2021, at 02:44, Corey Minyard wrote: > On Mon, May 10, 2021 at 03:12:02PM +0930, Andrew Jeffery wrote: > > Make the KCS device drivers responsible for allocating their own memory. > > > > Until now the private data for the device driver was allocated internal > > to the private data for the chardev interface. This coupling required > > the slightly awkward API of passing through the struct size for the > > driver private data to the chardev constructor, and then retrieving a > > pointer to the driver private data from the allocated chardev memory. > > > > In addition to being awkward, the arrangement prevents the > > implementation of alternative userspace interfaces as the device driver > > private data is not independent. > > > > Peel a layer off the onion and turn the data-structures inside out by > > exploiting container_of() and embedding `struct kcs_device` in the > > driver private data. > > All in all a very nice cleanup. A few nits inline. > > > > > Signed-off-by: Andrew Jeffery > > Reviewed-by: Zev Weiss > > --- > > drivers/char/ipmi/kcs_bmc.c | 19 +++++++-- > > drivers/char/ipmi/kcs_bmc.h | 12 ++---- > > drivers/char/ipmi/kcs_bmc_aspeed.c | 56 +++++++++++++------------ > > drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 60 ++++++++++++++++++--------- > > drivers/char/ipmi/kcs_bmc_npcm7xx.c | 37 ++++++++++------- > > 5 files changed, 111 insertions(+), 73 deletions(-) > > > > diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c > > index ef5c48ffe74a..83da681bf49e 100644 > > --- a/drivers/char/ipmi/kcs_bmc.c > > +++ b/drivers/char/ipmi/kcs_bmc.c > > @@ -44,12 +44,23 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc) > > } > > EXPORT_SYMBOL(kcs_bmc_handle_event); > > > > -struct kcs_bmc *kcs_bmc_ipmi_alloc(struct device *dev, int sizeof_priv, u32 channel); > > -struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv, u32 channel) > > +int kcs_bmc_ipmi_add_device(struct kcs_bmc *kcs_bmc); > > The above (and it's remove function) should be in an include file. This is a short-term hack while I'm refactoring the code. It goes away in a later patch when we switch to using an ops struct. I didn't move it to a header as it's an implementation detail at the end of the day. I see headers as describing a public interface, and in the bigger picture this function isn't part of the public API. But maybe it's too tricky by half. My approach here generated some discussion with Zev as well. > > > +void kcs_bmc_add_device(struct kcs_bmc *kcs_bmc) > > This should return an error so the probe can be failed and cleaned up > and so confusing message don't get printed after this in one case. Hmm. I did this because the end result of the series is that we can have multiple chardev interfaces in distinct modules exposing the one KCS device in the one kernel. If more than one of the chardev modules is configured in and one of them fails to initialise themselves with respect to the device driver I didn't think it was right to fail the probe of the device driver (and thus remove any chardev interfaces that did succeed to initialise against it). But this does limit the usefulness of the device driver instance in the case that only one of the chardev interfaces is configured in and it fails to initialise. So I think we need to decide on the direction before I adjust the interface here. The patches are architected around the idea of multiple chardevs being configured in to the kernel build and all are exposed at runtime. The serio subsystem does have the 'drvctl' sysfs knob that allows userspace to dictate which serio chardev interface they want to connect to a serio device driver. Maybe that's preferred over my "connect them all" strategy? Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel