From patchwork Mon Nov 27 19:22:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ladislav Michl X-Patchwork-Id: 10077961 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A8491602BD for ; Mon, 27 Nov 2017 19:22:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9564628AB8 for ; Mon, 27 Nov 2017 19:22:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8A76C28FB5; Mon, 27 Nov 2017 19:22:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 13C2228AB8 for ; Mon, 27 Nov 2017 19:22:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752897AbdK0TW5 (ORCPT ); Mon, 27 Nov 2017 14:22:57 -0500 Received: from eddie.linux-mips.org ([148.251.95.138]:57928 "EHLO cvs.linux-mips.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751475AbdK0TW4 (ORCPT ); Mon, 27 Nov 2017 14:22:56 -0500 Received: (from localhost user: 'ladis' uid#1021 fake: STDIN (ladis@eddie.linux-mips.org)) by eddie.linux-mips.org id S23990513AbdK0TWyN3loS (ORCPT + 3 others); Mon, 27 Nov 2017 20:22:54 +0100 Date: Mon, 27 Nov 2017 20:22:51 +0100 From: Ladislav Michl To: SF Markus Elfring Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, "Andrew F. Davis" , Arvind Yadav , Bartlomiej Zolnierkiewicz , Tomi Valkeinen , LKML , kernel-janitors@vger.kernel.org Subject: Re: omapfb/dss: Delete an error message for a failed memory allocation in three functions Message-ID: <20171127192251.GA23081@lenoch> References: <502de06b-ca86-d0ff-bd50-d260fbe46fe5@users.sourceforge.net> <20171127174454.GA18851@lenoch> <5e90409f-fcb0-cdb6-9bc3-26ad30a1f7c9@users.sourceforge.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5e90409f-fcb0-cdb6-9bc3-26ad30a1f7c9@users.sourceforge.net> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Nov 27, 2017 at 07:12:50PM +0100, SF Markus Elfring wrote: > >> Can a default allocation failure report provide the information > >> which you might expect so far? > > > > You should be able to answer that question yourself. > > I can not answer this detail completely myself because my knowledge > is incomplete about your concrete expectations for the exception handling > which can lead to different views for the need of additional error messages. The rule is to be able to get idea what failed without recompiling kernel. If you think you can point to failed allocation with your changes, then you might be able to convice Andrew your change is an improvement. > > And if you are unable to do so, just do not sent changes pointed > > by any code analysis tools. > > They can point aspects out for further software development considerations, > can't they? So? > > As a side note, if you look at whole call chain, you'll find quite some > > room for optimizations - look how dev and pdev are used. That also applies > > to other patches. > > Would you prefer to improve the source code in other ways? I would prefer you to look at code as a whole, not as an isolated set of lines which can be changes to shut up some random warning obtained from code analysis tool. Behold, here we saved over 100 bytes just by minor clean up. Patch is a mess, should be probably polished and splitted, but you'll get an idea. That said, we saved more than by deleting one error message and if you can prove we will not loose information _what_ failed with your patch, we can save even more. text data bss dec hex filename 59319 2372 4184 65875 10153 dispc.o.orig 59178 2372 4184 65734 100c6 dispc.o There is intentionally no signoff... --- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c index 7a75dfda9845..4c8463957634 100644 --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c @@ -3976,52 +3976,33 @@ static const struct dispc_features omap54xx_dispc_feats = { .has_writeback = true, }; -static int dispc_init_features(struct platform_device *pdev) +static const struct dispc_features* dispc_get_features(void) { - const struct dispc_features *src; - struct dispc_features *dst; - - dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL); - if (!dst) { - dev_err(&pdev->dev, "Failed to allocate DISPC Features\n"); - return -ENOMEM; - } - switch (omapdss_get_version()) { case OMAPDSS_VER_OMAP24xx: - src = &omap24xx_dispc_feats; - break; + return &omap24xx_dispc_feats; case OMAPDSS_VER_OMAP34xx_ES1: - src = &omap34xx_rev1_0_dispc_feats; - break; + return &omap34xx_rev1_0_dispc_feats; case OMAPDSS_VER_OMAP34xx_ES3: case OMAPDSS_VER_OMAP3630: case OMAPDSS_VER_AM35xx: case OMAPDSS_VER_AM43xx: - src = &omap34xx_rev3_0_dispc_feats; - break; + return &omap34xx_rev3_0_dispc_feats; case OMAPDSS_VER_OMAP4430_ES1: case OMAPDSS_VER_OMAP4430_ES2: case OMAPDSS_VER_OMAP4: - src = &omap44xx_dispc_feats; - break; + return &omap44xx_dispc_feats; case OMAPDSS_VER_OMAP5: case OMAPDSS_VER_DRA7xx: - src = &omap54xx_dispc_feats; - break; + return &omap54xx_dispc_feats; default: - return -ENODEV; + return NULL; } - - memcpy(dst, src, sizeof(*dst)); - dispc.feat = dst; - - return 0; } static irqreturn_t dispc_irq_handler(int irq, void *arg) @@ -4068,54 +4049,61 @@ EXPORT_SYMBOL(dispc_free_irq); /* DISPC HW IP initialisation */ static int dispc_bind(struct device *dev, struct device *master, void *data) { - struct platform_device *pdev = to_platform_device(dev); u32 rev; int r = 0; + const struct dispc_features *features; struct resource *dispc_mem; - struct device_node *np = pdev->dev.of_node; + struct device_node *np = dev->of_node; - dispc.pdev = pdev; + dispc.pdev = to_platform_device(dev); spin_lock_init(&dispc.control_lock); - r = dispc_init_features(dispc.pdev); - if (r) - return r; + features = dispc_get_features(); + if (!features) + return -ENODEV; + + dispc.feat = devm_kzalloc(dev, sizeof(*features), GFP_KERNEL); + if (dispc.feat) { + dev_err(dev, "Failed to allocate DISPC Features\n"); + return -ENOMEM; + } + memcpy(dispc.feat, features, sizeof(*features)); dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0); if (!dispc_mem) { - DSSERR("can't get IORESOURCE_MEM DISPC\n"); + dev_err(dev, "can't get IORESOURCE_MEM DISPC\n"); return -EINVAL; } - dispc.base = devm_ioremap(&pdev->dev, dispc_mem->start, + dispc.base = devm_ioremap(dev, dispc_mem->start, resource_size(dispc_mem)); if (!dispc.base) { - DSSERR("can't ioremap DISPC\n"); + dev_err(dev, "can't ioremap DISPC\n"); return -ENOMEM; } dispc.irq = platform_get_irq(dispc.pdev, 0); if (dispc.irq < 0) { - DSSERR("platform_get_irq failed\n"); + dev_err(dev, "platform_get_irq failed\n"); return -ENODEV; } if (np && of_property_read_bool(np, "syscon-pol")) { dispc.syscon_pol = syscon_regmap_lookup_by_phandle(np, "syscon-pol"); if (IS_ERR(dispc.syscon_pol)) { - dev_err(&pdev->dev, "failed to get syscon-pol regmap\n"); + dev_err(dev, "failed to get syscon-pol regmap\n"); return PTR_ERR(dispc.syscon_pol); } if (of_property_read_u32_index(np, "syscon-pol", 1, &dispc.syscon_pol_offset)) { - dev_err(&pdev->dev, "failed to get syscon-pol offset\n"); + dev_err(dev, "failed to get syscon-pol offset\n"); return -EINVAL; } } - pm_runtime_enable(&pdev->dev); + pm_runtime_enable(dev); r = dispc_runtime_get(); if (r) @@ -4124,7 +4112,7 @@ static int dispc_bind(struct device *dev, struct device *master, void *data) _omap_dispc_initial_config(); rev = dispc_read_reg(DISPC_REVISION); - dev_dbg(&pdev->dev, "OMAP DISPC rev %d.%d\n", + dev_dbg(dev, "OMAP DISPC rev %d.%d\n", FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0)); dispc_runtime_put(); @@ -4136,7 +4124,7 @@ static int dispc_bind(struct device *dev, struct device *master, void *data) return 0; err_runtime_get: - pm_runtime_disable(&pdev->dev); + pm_runtime_disable(dev); return r; }