Received: by 10.223.164.202 with SMTP id h10csp2070703wrb; Mon, 27 Nov 2017 11:25:31 -0800 (PST) X-Google-Smtp-Source: AGs4zMZnI6AlmifJybkPm4Ccg3mOhsk6TpuOhADOx1Yrh4I7+/RaHmF2TT3dTWoCrdupogHBM+Tx X-Received: by 10.98.27.3 with SMTP id b3mr37930670pfb.159.1511810731655; Mon, 27 Nov 2017 11:25:31 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511810731; cv=none; d=google.com; s=arc-20160816; b=aSM56Aqdn0hTgIEfd8XWAHEAzkZTIGnsaETbeldqj6XKlJkqP7sut4xtc1RK//1tWm /9LqEW0UcW/I8GcXuYejPTRazYhJHnUwtt30ZhGIzElz33zcBSpvHQgeb8JbJ29hWaRK +lVduLMpnFXZCjNfaWXV9kEoo+J56ePLwOOwg1s30+248lmH68Q9PTqgIUSH65lcaHdf ilgtGiNqlvKei60Y1/GSc1oR8yTv4ayw7q/UQv5+WlBHER6PTlkPG7n6VQS2VJfuPb+x JqE5VDWqbx6g9BM7KF8CTbP9G2pYeNZmXoqEv8O5FOGRGGrMrUnJnjaypgalWQjx/NwA 1u9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:arc-authentication-results; bh=d4ctBv6pc/DqIt+sNg37zQO1ZfLFSzPu0K47jjaPD9c=; b=Lce3e1c8ujCOD/Qx/EmHeks9l9YuYfA2iDjSAU54Vy/hiL6FdgspN62LKn7iujsKvx xsBcIPrYoz/VnUBtx3P7I648MnP7yDlP99QkKCcdgMJYwhkn/+hQNihE/aRtqR9xgxrG 6tHwcYlEir88DLA6IRAzjPTjsdvS2M4nosjS9B6GQZ50QewD6ifT/qfTsGXRdUHguDd/ 49uT4KQ6jnndyvk9qwP45qERTOtBxgik9TmVmBHQY86T2NDSR2zL2RjNMcdaTgbpOaUs 3koqYHFgmpJLkdVTq0PY664psIUZmicFs+/yFgznly7ajCjg9WYafwj1JXwbrrrS3lBb +ryA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h82si26147750pfa.196.2017.11.27.11.25.19; Mon, 27 Nov 2017 11:25:31 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753409AbdK0TW6 (ORCPT + 79 others); Mon, 27 Nov 2017 14:22:58 -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-Type: text/plain; charset=us-ascii 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-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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... 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; } From 1585247356913026723@xxx Mon Nov 27 19:08:09 +0000 2017 X-GM-THRID: 1585156039807309629 X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread