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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 A2519C433EF for ; Mon, 13 Jun 2022 05:53:34 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4F3FF89269; Mon, 13 Jun 2022 05:53:31 +0000 (UTC) Received: from mailrelay1-1.pub.mailoutpod1-cph3.one.com (mailrelay1-1.pub.mailoutpod1-cph3.one.com [46.30.210.182]) by gabe.freedesktop.org (Postfix) with ESMTPS id DA7EB10E66E for ; Mon, 13 Jun 2022 05:53:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ravnborg.org; s=rsa1; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=lviILeZfMx47/T5mT57g5CF3Go1+RnlpuLxpXoQJAzs=; b=sUYyi0kX/qm8dYiqVBEu30Ua03s1/pEag+lIBuG61+eOdPZ0ibmZ8L0jC+Mbw9yLJkxXoH1rFwvnY IppPwkQoYAL2aKyaU6xPHLjOQvqjDW87zkWVzetmd8eoBw0ztD+1nwLVHJQnCbdjwS2jYX2+1zAfoN qdZDfyAfIM419tB7J3rikYpJiHi7HKv8JXlhBxyb/XAIPXpwZ3GGIdVPINGD+N41VjuI+vAup3aom3 YjaDyc5yTGH0ptGOp30T8PCxQgimrWbTiYLlODVBtJXzAl/ApAbLxTxbWuVEhRQy/MeTC3o8dOyv2a 65tqVAIKPJHAVKK4pA3XHh+4cL23G6w== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ravnborg.org; s=ed1; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=lviILeZfMx47/T5mT57g5CF3Go1+RnlpuLxpXoQJAzs=; b=X74rpjvF9v21uUYO5EGbAEcnnWsBe1DgVlkBhT2gw+Uak7jOwZ/GSdKzOSsHmluG6E9lCcsNJO/Jy reK92zBBQ== X-HalOne-Cookie: 4e71c7d0b0e443bd93c3ca12317c244ea4f8fa6a X-HalOne-ID: 24fddb4a-eadd-11ec-a6c1-d0431ea8a283 Received: from mailproxy3.cst.dirpod3-cph3.one.com (80-162-45-141-cable.dk.customer.tdc.net [80.162.45.141]) by mailrelay1.pub.mailoutpod1-cph3.one.com (Halon) with ESMTPSA id 24fddb4a-eadd-11ec-a6c1-d0431ea8a283; Mon, 13 Jun 2022 05:53:27 +0000 (UTC) Date: Mon, 13 Jun 2022 07:53:25 +0200 From: Sam Ravnborg To: Laurent Pinchart Subject: Re: [LINUX PATCH 2/2] drm: xlnx: dsi: driver for Xilinx DSI Tx subsystem Message-ID: References: <1652363593-45799-1-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com> <1652363593-45799-3-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: airlied@linux.ie, vgannava@xilinx.com, Venkateshwar Rao Gannavarapu , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Laurent, > [snip] > > > > +static inline void xlnx_dsi_writel(void __iomem *base, int offset, u32 val) > > > +{ > > > + writel(val, base + offset); > > > +} > > > + > > > +static inline u32 xlnx_dsi_readl(void __iomem *base, int offset) > > > +{ > > > + return readl(base + offset); > > > +} > > > > When I see implementations like this I wonder if a regmap would be > > beneficial? > > regmap often seems overkill to me when the driver only needs plain > 32-bit mmio read/write, given the overhead it adds at runtime. Is it > just me ? There are several points that speaks for using regmap: - The interface is well known - It has nice helpers - like update_bits - No need for own wrappers, that sometimes are made in creative ways (not the case here) - There is a possibility to add some run-time checks so one can catch attempt to write outside the register window, write to read-only registers etc. On top of this - it is simple to configure: static const struct regmap_config regmap_config = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, }; >From the probe function: priv->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(priv->regs)) return dev_err_probe(dev, PTR_ERR(priv->regs), "Failed to get memory resource\n"); regmap_cfg = regmap_config; regmap_cfg.max_register = res->end - res->start; priv->regmap = devm_regmap_init_mmio(dev, priv->regs, ®map_cfg); if (IS_ERR(priv->regmap)) return dev_err_probe(dev, PTR_ERR(priv->regmap), "Failed to init regmap\n"); The one point that brought me over was the well known interface. But using wrappers works too. Sam 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 BDA76C433EF for ; Mon, 13 Jun 2022 05:53:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236517AbiFMFxp (ORCPT ); Mon, 13 Jun 2022 01:53:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235807AbiFMFxe (ORCPT ); Mon, 13 Jun 2022 01:53:34 -0400 Received: from mailrelay1-1.pub.mailoutpod1-cph3.one.com (mailrelay1-1.pub.mailoutpod1-cph3.one.com [46.30.210.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 08E47DF3E for ; Sun, 12 Jun 2022 22:53:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ravnborg.org; s=rsa1; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=lviILeZfMx47/T5mT57g5CF3Go1+RnlpuLxpXoQJAzs=; b=sUYyi0kX/qm8dYiqVBEu30Ua03s1/pEag+lIBuG61+eOdPZ0ibmZ8L0jC+Mbw9yLJkxXoH1rFwvnY IppPwkQoYAL2aKyaU6xPHLjOQvqjDW87zkWVzetmd8eoBw0ztD+1nwLVHJQnCbdjwS2jYX2+1zAfoN qdZDfyAfIM419tB7J3rikYpJiHi7HKv8JXlhBxyb/XAIPXpwZ3GGIdVPINGD+N41VjuI+vAup3aom3 YjaDyc5yTGH0ptGOp30T8PCxQgimrWbTiYLlODVBtJXzAl/ApAbLxTxbWuVEhRQy/MeTC3o8dOyv2a 65tqVAIKPJHAVKK4pA3XHh+4cL23G6w== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ravnborg.org; s=ed1; h=in-reply-to:content-type:mime-version:references:message-id:subject:cc:to: from:date:from; bh=lviILeZfMx47/T5mT57g5CF3Go1+RnlpuLxpXoQJAzs=; b=X74rpjvF9v21uUYO5EGbAEcnnWsBe1DgVlkBhT2gw+Uak7jOwZ/GSdKzOSsHmluG6E9lCcsNJO/Jy reK92zBBQ== X-HalOne-Cookie: 4e71c7d0b0e443bd93c3ca12317c244ea4f8fa6a X-HalOne-ID: 24fddb4a-eadd-11ec-a6c1-d0431ea8a283 Received: from mailproxy3.cst.dirpod3-cph3.one.com (80-162-45-141-cable.dk.customer.tdc.net [80.162.45.141]) by mailrelay1.pub.mailoutpod1-cph3.one.com (Halon) with ESMTPSA id 24fddb4a-eadd-11ec-a6c1-d0431ea8a283; Mon, 13 Jun 2022 05:53:27 +0000 (UTC) Date: Mon, 13 Jun 2022 07:53:25 +0200 From: Sam Ravnborg To: Laurent Pinchart Cc: Venkateshwar Rao Gannavarapu , airlied@linux.ie, vgannava@xilinx.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [LINUX PATCH 2/2] drm: xlnx: dsi: driver for Xilinx DSI Tx subsystem Message-ID: References: <1652363593-45799-1-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com> <1652363593-45799-3-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Laurent, > [snip] > > > > +static inline void xlnx_dsi_writel(void __iomem *base, int offset, u32 val) > > > +{ > > > + writel(val, base + offset); > > > +} > > > + > > > +static inline u32 xlnx_dsi_readl(void __iomem *base, int offset) > > > +{ > > > + return readl(base + offset); > > > +} > > > > When I see implementations like this I wonder if a regmap would be > > beneficial? > > regmap often seems overkill to me when the driver only needs plain > 32-bit mmio read/write, given the overhead it adds at runtime. Is it > just me ? There are several points that speaks for using regmap: - The interface is well known - It has nice helpers - like update_bits - No need for own wrappers, that sometimes are made in creative ways (not the case here) - There is a possibility to add some run-time checks so one can catch attempt to write outside the register window, write to read-only registers etc. On top of this - it is simple to configure: static const struct regmap_config regmap_config = { .reg_bits = 32, .val_bits = 32, .reg_stride = 4, }; >From the probe function: priv->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(priv->regs)) return dev_err_probe(dev, PTR_ERR(priv->regs), "Failed to get memory resource\n"); regmap_cfg = regmap_config; regmap_cfg.max_register = res->end - res->start; priv->regmap = devm_regmap_init_mmio(dev, priv->regs, ®map_cfg); if (IS_ERR(priv->regmap)) return dev_err_probe(dev, PTR_ERR(priv->regmap), "Failed to init regmap\n"); The one point that brought me over was the well known interface. But using wrappers works too. Sam