From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751088AbdBHSWw (ORCPT ); Wed, 8 Feb 2017 13:22:52 -0500 Received: from mail-qt0-f196.google.com ([209.85.216.196]:35001 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750941AbdBHSWv (ORCPT ); Wed, 8 Feb 2017 13:22:51 -0500 Subject: Re: [PATCH] net: phy: dp83867: Fall-back to default values of clock delay and FIFO depth To: David Miller , Alexey.Brodkin@synopsys.com References: <20170206192445.35829-1-abrodkin@synopsys.com> <20170208.131519.1313210815958231832.davem@davemloft.net> Cc: netdev@vger.kernel.org, linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org, m-karicheri2@ti.com, nsekhar@ti.com, grygorii.strashko@ti.com, mugunthanvnm@ti.com, andrew@lunn.ch From: Florian Fainelli Message-ID: <360cfba0-cc84-4378-025d-34e5eb1fc2c8@gmail.com> Date: Wed, 8 Feb 2017 10:20:00 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <20170208.131519.1313210815958231832.davem@davemloft.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/08/2017 10:15 AM, David Miller wrote: > From: Alexey Brodkin > Date: Mon, 6 Feb 2017 22:24:45 +0300 > >> Given there're default values mentioned in the PHY datasheet >> fall-back gracefully to them instead of silently return an error >> through the whole call-chain. >> >> This allows to use minimalistic description in DT if no special >> features are required. >> >> Signed-off-by: Alexey Brodkin > > If these defaults are legitimate to use, then you should probably also > set them in the non-CONFIG_OF_MDIO case implementation of > dp83867_of_init(). IIRC this is what Alexey is doing already, in case of errors, instead of returning the error code, he changes the default values and does not propagate the error code. Alexey, you are essentially making dp83867_of_init() impossible to fail (or nearly) now, even with invalid properties, so I agree with David here, you should probably have a function that runs after dp83867_of_init() whose job is to set default values, if none have been previously set through Device Tree. > > But I'd like to see feedback and reviews from other PHY area experts > before I even consider that updated version of this change. > > Thanks. > -- Florian