From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754815AbcEZTAQ (ORCPT ); Thu, 26 May 2016 15:00:16 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:34897 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751346AbcEZTAN (ORCPT ); Thu, 26 May 2016 15:00:13 -0400 Date: Thu, 26 May 2016 21:00:00 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Linus Walleij Cc: Sergei Shtylyov , Grant Likely , Rob Herring , "devicetree@vger.kernel.org" , Florian Fainelli , "netdev@vger.kernel.org" , Frank Rowand , =?utf-8?B?UGF3ZcWC?= Moll , Mark Rutland , "ijc+devicetree@hellion.org.uk" , Kumar Gala , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH RFT 1/2] phylib: add device reset GPIO support Message-ID: <20160526190000.GE3331@pengutronix.de> References: <81129033.NXiOLTg1so@wasted.cogentembedded.com> <3641492.klKRrvS8tr@wasted.cogentembedded.com> <20160512184233.GJ30822@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c5 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 26, 2016 at 11:00:55AM +0200, Linus Walleij wrote: > On Thu, May 12, 2016 at 8:42 PM, Uwe Kleine-König > wrote: > > > [added Linus Walleij to Cc, there is a question for you/him below] > (...) > >> +void mdio_device_reset(struct mdio_device *mdiodev, int value) > >> +{ > >> + if (mdiodev->reset) > >> + gpiod_set_value(mdiodev->reset, value); > > > > Before v4.6-rc1~108^2~91 it was not necessary to check for the first > > parameter being non-NULL before calling gpiod_set_value. Linus, did you > > change this on purpose? > > Not really. And AFAICT it is still not necessary: what changed is that > an error message will be printed by VALIDATE_DESC() if you do that. > And that is proper I guess? I think it's sloppy code to randomly pass in > NULL to a call and just expect it to bail out, it seems more like > exercising the error path than something you'd normally rely on. > > Or am I getting things wrong? is the following sloppy?: somegpio = gpiod_get_optional(dev, "some", GPIOD_OUT_LOW); if (IS_ERR(somegpio)) return PTR_ERR(somegpio); gpiod_set_value(somegpio, 1); If not (as I assume) you really changed something as this might trigger the warning. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |