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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDA25C433EF for ; Wed, 6 Oct 2021 12:01:39 +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 82B12610A3 for ; Wed, 6 Oct 2021 12:01:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 82B12610A3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=acm.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=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: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=irpZUXprhHtX7ljMLZCXtw08PL1Q/rASRA2OYurW/ig=; b=HqtlurR8ue4pTk 7Vgi/X0GoTwf0B9WP/oU9OGX705f4zu/lhIVPtjYb4jjjA/n85bUxeHhD9BTENrx60rHGYD8bZNxn FeEMdYk9LyqrlxfSLdAprtgTtMpxkbtc77DdKYP4HviyfDUQf0ajdXCE49yNgzQg5BRtgDfSzPqZJ 1eu9AJK9v1WsAouhtL34hWXULZRud7ssN22EF3o2hWYus2LZl1M2Ofe4biwqgeASQw81rVILw8948 Re8Qg9cmGgyMSQV8TuQ5VY3VlGKcrVsnJKFZ1A4slv0IHMSr3pVKnuaLVldqOD8hZ602vbYf2LbH0 zkc6RMot+A4hEg1eqACA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mY5aR-00E9HH-38; Wed, 06 Oct 2021 11:59:51 +0000 Received: from mail-ot1-x32b.google.com ([2607:f8b0:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mY5aM-00E9Gx-W7 for linux-arm-kernel@lists.infradead.org; Wed, 06 Oct 2021 11:59:48 +0000 Received: by mail-ot1-x32b.google.com with SMTP id j11-20020a9d190b000000b00546fac94456so2746359ota.6 for ; Wed, 06 Oct 2021 04:59:45 -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=MVq/g3Xc5jAf1h7m4YuFHQqvxl5RNLco0ZW8S3JTUc8=; b=Rnt1jqsImTtS5OIpmDsJ8MBxSSb8TX+SRqw8yb6guOXCxZTjmwWMINMPtDfbMck17m A1p+CEO3u8r4lLa+0Y/Nl+rODKh9GaiakC17uehMxgazVUCvgIaIm3Cq90eK/WYijs71 L4fPZQE8dGEEYyzyf0r6njR2w2eGOF8u5Ho6VyQ4xpwmXOKRNG+2Rb5hlNUO8fIBhsPq to0secdOCbCWKnI++IAQyFBk5RYue65DpEKX5WGH+10pkMb8YtIDoVvbL0lG0XgvV9wF iCjPiLCqDZXZ9hMMSnZa8iCS3fX1IJYgRexlNkSAHqlwT8M3e4uH13AW6MaOrvaQaLNL xAqg== 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=MVq/g3Xc5jAf1h7m4YuFHQqvxl5RNLco0ZW8S3JTUc8=; b=nFxK/SpXuOqHPy6gdAfgkk4W5rpnIfipCutgwgy/ZslAZ2eQtz76DZu873ARLhmap/ RbVM/ZZA4UVyoOKXic9FzaQhokSFFcvRh773fsCeO8OtpL1vHO+xEfDL25JT29xKSpIN G6nOniwgzQGgNNQrHDZS5MuSlMIgcpQsSW9wKXW3l1jlaX9v1MPO48hs6PD0jmi1XPw3 m92ZBSjEfXe5ODWDOyYyqbps6aj7OzmRZf+oTlbTTyU0F/BUd84a1himrnTgRyRG0HTr G5N83IjNgN8J+7jjGDxxcyyS1Sh0MQt9EKWixDkfdDBfwAMRnodZrlJ+VFd+BBnKixpR CYrw== X-Gm-Message-State: AOAM531b1j92RbW6wruYG+xXrGOJiUfHMH8tNpJisPK5zFj7BbhyQ3yf o30nZz+z0wg3Kh8M6/Dk4fXrslYGOg== X-Google-Smtp-Source: ABdhPJzWgcICAB+naoiE4NIjYdfqwgqzfK1aU/SCFKRg/uW1IhOSBNp5T2UMwn1j28I+UoLf5DaVqw== X-Received: by 2002:a05:6830:4387:: with SMTP id s7mr1440191otv.273.1633521584953; Wed, 06 Oct 2021 04:59:44 -0700 (PDT) Received: from serve.minyard.net ([47.184.156.158]) by smtp.gmail.com with ESMTPSA id e10sm3908495oig.11.2021.10.06.04.59.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Oct 2021 04:59:44 -0700 (PDT) Received: from minyard.net (unknown [IPv6:2001:470:b8f6:1b:d981:37c0:4da7:a7f4]) by serve.minyard.net (Postfix) with ESMTPSA id 89EE21800ED; Wed, 6 Oct 2021 11:59:43 +0000 (UTC) Date: Wed, 6 Oct 2021 06:59:42 -0500 From: Corey Minyard To: Joel Stanley Cc: openipmi-developer@lists.sourceforge.net, Linux ARM , Andrew Jeffery , Alistar Popple Subject: Re: [PATCH] ipmi: bt-bmc: Use registers directly Message-ID: <20211006115942.GI5381@minyard.net> References: <20210903051039.307991-1-joel@jms.id.au> 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-20211006_045947_096235_24354E6A X-CRM114-Status: GOOD ( 37.03 ) 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, Oct 06, 2021 at 02:33:06AM +0000, Joel Stanley wrote: > Hi Corey, > > On Fri, 3 Sept 2021 at 05:10, Joel Stanley wrote: > > > > This driver was originally written to use the regmap abstraction with no > > clear benefit. As the registers are always mmio and there is no sharing > > of the region with other devices, we can safely read and write without > > the locking that regmap provides. > > > > This reduces the code size of the driver by about 25%. > > Do you have any feedback on this one? Ah, sorry, this looks ok to me. I was unable to find much useful information about the benefits of regmap, but it seems that if you had registers layed out in various ways, with different caching, etc, that regmap would be useful. It might be useful for the host side driver because it deals with various register layouts. However, for this application, it doesn't seem useful. I didn't see any other comments on it, but it looks clean. Applied to my next tree. -corey > > Cheers, > > Joel > > > > > > Signed-off-by: Joel Stanley > > --- > > drivers/char/ipmi/bt-bmc.c | 68 +++++++++----------------------------- > > 1 file changed, 16 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c > > index 6e3d247b55d1..fb771ce6f7bf 100644 > > --- a/drivers/char/ipmi/bt-bmc.c > > +++ b/drivers/char/ipmi/bt-bmc.c > > @@ -8,13 +8,11 @@ > > #include > > #include > > #include > > -#include > > #include > > #include > > #include > > #include > > #include > > -#include > > #include > > #include > > > > @@ -59,8 +57,7 @@ > > struct bt_bmc { > > struct device dev; > > struct miscdevice miscdev; > > - struct regmap *map; > > - int offset; > > + void __iomem *base; > > int irq; > > wait_queue_head_t queue; > > struct timer_list poll_timer; > > @@ -69,29 +66,14 @@ struct bt_bmc { > > > > static atomic_t open_count = ATOMIC_INIT(0); > > > > -static const struct regmap_config bt_regmap_cfg = { > > - .reg_bits = 32, > > - .val_bits = 32, > > - .reg_stride = 4, > > -}; > > - > > static u8 bt_inb(struct bt_bmc *bt_bmc, int reg) > > { > > - uint32_t val = 0; > > - int rc; > > - > > - rc = regmap_read(bt_bmc->map, bt_bmc->offset + reg, &val); > > - WARN(rc != 0, "regmap_read() failed: %d\n", rc); > > - > > - return rc == 0 ? (u8) val : 0; > > + return readb(bt_bmc->base + reg); > > } > > > > static void bt_outb(struct bt_bmc *bt_bmc, u8 data, int reg) > > { > > - int rc; > > - > > - rc = regmap_write(bt_bmc->map, bt_bmc->offset + reg, data); > > - WARN(rc != 0, "regmap_write() failed: %d\n", rc); > > + writeb(data, bt_bmc->base + reg); > > } > > > > static void clr_rd_ptr(struct bt_bmc *bt_bmc) > > @@ -376,18 +358,15 @@ static irqreturn_t bt_bmc_irq(int irq, void *arg) > > { > > struct bt_bmc *bt_bmc = arg; > > u32 reg; > > - int rc; > > > > - rc = regmap_read(bt_bmc->map, bt_bmc->offset + BT_CR2, ®); > > - if (rc) > > - return IRQ_NONE; > > + reg = readl(bt_bmc->base + BT_CR2); > > > > reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY; > > if (!reg) > > return IRQ_NONE; > > > > /* ack pending IRQs */ > > - regmap_write(bt_bmc->map, bt_bmc->offset + BT_CR2, reg); > > + writel(reg, bt_bmc->base + BT_CR2); > > > > wake_up(&bt_bmc->queue); > > return IRQ_HANDLED; > > @@ -398,6 +377,7 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc, > > { > > struct device *dev = &pdev->dev; > > int rc; > > + u32 reg; > > > > bt_bmc->irq = platform_get_irq_optional(pdev, 0); > > if (bt_bmc->irq < 0) > > @@ -417,11 +397,11 @@ static int bt_bmc_config_irq(struct bt_bmc *bt_bmc, > > * will be cleared (along with B2H) when we can write the next > > * message to the BT buffer > > */ > > - rc = regmap_update_bits(bt_bmc->map, bt_bmc->offset + BT_CR1, > > - (BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY), > > - (BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY)); > > + reg = readl(bt_bmc->base + BT_CR1); > > + reg |= BT_CR1_IRQ_H2B | BT_CR1_IRQ_HBUSY; > > + writel(reg, bt_bmc->base + BT_CR1); > > > > - return rc; > > + return 0; > > } > > > > static int bt_bmc_probe(struct platform_device *pdev) > > @@ -439,25 +419,9 @@ static int bt_bmc_probe(struct platform_device *pdev) > > > > dev_set_drvdata(&pdev->dev, bt_bmc); > > > > - bt_bmc->map = syscon_node_to_regmap(pdev->dev.parent->of_node); > > - if (IS_ERR(bt_bmc->map)) { > > - void __iomem *base; > > - > > - /* > > - * Assume it's not the MFD-based devicetree description, in > > - * which case generate a regmap ourselves > > - */ > > - base = devm_platform_ioremap_resource(pdev, 0); > > - if (IS_ERR(base)) > > - return PTR_ERR(base); > > - > > - bt_bmc->map = devm_regmap_init_mmio(dev, base, &bt_regmap_cfg); > > - bt_bmc->offset = 0; > > - } else { > > - rc = of_property_read_u32(dev->of_node, "reg", &bt_bmc->offset); > > - if (rc) > > - return rc; > > - } > > + bt_bmc->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(bt_bmc->base)) > > + return PTR_ERR(bt_bmc->base); > > > > mutex_init(&bt_bmc->mutex); > > init_waitqueue_head(&bt_bmc->queue); > > @@ -483,12 +447,12 @@ static int bt_bmc_probe(struct platform_device *pdev) > > add_timer(&bt_bmc->poll_timer); > > } > > > > - regmap_write(bt_bmc->map, bt_bmc->offset + BT_CR0, > > - (BT_IO_BASE << BT_CR0_IO_BASE) | > > + writel((BT_IO_BASE << BT_CR0_IO_BASE) | > > (BT_IRQ << BT_CR0_IRQ) | > > BT_CR0_EN_CLR_SLV_RDP | > > BT_CR0_EN_CLR_SLV_WRP | > > - BT_CR0_ENABLE_IBT); > > + BT_CR0_ENABLE_IBT, > > + bt_bmc->base + BT_CR0); > > > > clr_b_busy(bt_bmc); > > > > -- > > 2.33.0 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel