From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752627AbcHIJ3L (ORCPT ); Tue, 9 Aug 2016 05:29:11 -0400 Received: from mail-wm0-f42.google.com ([74.125.82.42]:38535 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752019AbcHIJ24 (ORCPT ); Tue, 9 Aug 2016 05:28:56 -0400 Date: Tue, 9 Aug 2016 10:30:22 +0100 From: Lee Jones To: SF Markus Elfring Cc: LKML , kernel-janitors@vger.kernel.org, Julia Lawall Subject: Re: [PATCH v4] mfd: dm355evm_msp: Refactoring for add_child() Message-ID: <20160809093022.GU5243@dell> References: <4dcb4cab-e2cc-87c0-9cfc-d140f185254b@users.sourceforge.net> <20160805075511.GN5243@dell> <0a2ef320-a95a-2611-2554-a2a83838fb9b@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0a2ef320-a95a-2611-2554-a2a83838fb9b@users.sourceforge.net> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 08 Aug 2016, SF Markus Elfring wrote: > >> v4: Further feedback was integrated into this message. > > > > This is not a good change-log. What actually changed? > > Which kind of information would you find more useful in this case? That's for you to tell me surely? If I wanted to know what changed, I would normally look at the patch change-log. But the change-log in this patch says "I did some stuff". What stuff did you change? Which review comments did you tend to? > > >> @@ -222,19 +222,20 @@ static struct device *add_child(struct i2c_client *client, const char *name, > >> status = platform_device_add_resources(pdev, &r, 1); > >> if (status < 0) { > >> dev_dbg(&pdev->dev, "can't add irq\n"); > >> - goto err; > >> + goto put_device; > >> } > >> } > >> > >> status = platform_device_add(pdev); > >> + if (status) > >> + goto put_device; > >> > >> -err: > >> - if (status < 0) { > >> - platform_device_put(pdev); > >> - dev_err(&client->dev, "can't add %s dev\n", name); > >> - return ERR_PTR(status); > >> - } > >> return &pdev->dev; > >> + > >> +put_device: > >> + platform_device_put(pdev); > >> + dev_err(&client->dev, "failed to add device %s\n", name); > > > > ... and remove this line. > > -Do you really want that this error message should be deleted? > > How does this response fit to your request to introduce such a message > for the function "add_numbered_child" (on 2016-06-08)? > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1162299.html > https://lkml.org/lkml/2016/6/8/467 You've lost the context. The "..." is meant to intimate that it follows on from a previous comment. In this case: > > status = platform_device_add_data(pdev, pdata, pdata_len); > > if (status < 0) { > > dev_dbg(&pdev->dev, "can't add platform_data\n"); > > Please take the opportunity to convert these to dev_err()s. So, convert the specific dev_dbg() calls to dev_err() and remove the contentless one at the bottom. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog